Perflib fixes (Windows.plugin) - #21458
Merged
thiagoftsm merged 18 commits intoDec 15, 2025
Merged
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
7 issues found across 12 files
Prompt for AI agents (all 7 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/collectors/windows.plugin/perflib-web-service.c">
<violation number="1" location="src/collectors/windows.plugin/perflib-web-service.c:453">
P2: Typo: 'attemptts' should be 'attempts' (double 't' introduced).</violation>
<violation number="2" location="src/collectors/windows.plugin/perflib-web-service.c:460">
P2: Typo: 'attemptts' should be 'attempts' in the chart context.</violation>
<violation number="3" location="src/collectors/windows.plugin/perflib-web-service.c:461">
P2: Typo: 'attemptts' should be 'attempts' in the chart title.</violation>
<violation number="4" location="src/collectors/windows.plugin/perflib-web-service.c:666">
P2: Typo: 'attemptts' should be 'attempts' (double 't' introduced).</violation>
<violation number="5" location="src/collectors/windows.plugin/perflib-web-service.c:673">
P2: Typo: 'attemptts' should be 'attempts' in the chart context.</violation>
<violation number="6" location="src/collectors/windows.plugin/perflib-web-service.c:674">
P2: Typo: 'attemptts' should be 'attempts' in the chart title.</violation>
</file>
<file name="src/collectors/windows.plugin/perflib-objects.c">
<violation number="1" location="src/collectors/windows.plugin/perflib-objects.c:48">
P0: Inverted condition: returns error (-1) on success and success (0) on failure. The condition should be negated since `do_objects` returns `true` on success and `false` on failure.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
Sorry, something went wrong.
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-web-service.c
Outdated
Comment thread
src/collectors/windows.plugin/perflib-objects.c
Outdated
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes various incorrect perflib mappings, labels, priorities, and typos across multiple Windows plugin collectors to improve the accuracy and reliability of metrics collection.
Key changes:
- Corrected counter key mappings and variable names in ADFS, ASP.NET, Exchange, .NET Framework, Network, Memory, and IIS collectors
- Fixed naming consistency issues (e.g., "attemp" → "attempt", "WORKlOAD" → "WORKLOAD")
- Improved code quality by removing unused initialization code, fixing type casts, and correcting buffer sizes
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file| File | Description |
|---|---|
| src/collectors/windows.plugin/windows_plugin.h | Fixed typo in enum name (ATTEMP → ATTEMPT) and corrected Exchange WORKLOAD priority enum names |
| src/collectors/windows.plugin/perflib-web-service.c | Renamed variables and functions from "attemp/attemps" to "attempt/attempts", fixed counter source for connection attempts, corrected chart ID to use app_name instead of windows_shared_buffer |
| src/collectors/windows.plugin/perflib-storage.c | Increased volumeName buffer size from 260 to 261 bytes and updated GetVolumeInformationA size parameter accordingly |
| src/collectors/windows.plugin/perflib-processes.c | Removed unused empty initialize() function and its invocation |
| src/collectors/windows.plugin/perflib-objects.c | Added explicit type casts for semaphore metrics and fixed return value checking for do_objects() |
| src/collectors/windows.plugin/perflib-numa.c | Fixed chart ID prefix typo (numae → numa) and corrected counter reading logic to always update charts |
| src/collectors/windows.plugin/perflib-network.c | Fixed OutEchoReps counter key for both ICMPv4 and ICMPv6 from "Received" to "Sent" |
| src/collectors/windows.plugin/perflib-netframework.c | Fixed remoting counter reference, corrected variable names for CLR security checks (link_time → rt_checks, stack_walk_depth → run_time_checks), and fixed dimension ID for queue length |
| src/collectors/windows.plugin/perflib-memory.c | Corrected counter key names from "/s" to "/sec" and removed extraneous line breaks in formatting |
| src/collectors/windows.plugin/perflib-exchange.c | Fixed typo in variable name (Deliery → Delivery), function name (worload → workload), corrected counter usage for RPC active user count, and updated WORKLOAD priority enum names |
| src/collectors/windows.plugin/perflib-asp.c | Fixed counter references for executing requests and transaction functions |
| src/collectors/windows.plugin/perflib-adfs.c | Swapped OAuth password grant and token request counter key assignments, and adjusted priorities for password grant and token request functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
src/collectors/windows.plugin/perflib-storage.c
Outdated
stelfrag
reviewed
Dec 15, 2025
Comment thread
src/collectors/windows.plugin/GetSensors.c
Outdated
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
src/collectors/windows.plugin/GetSensors.c
Outdated
stelfrag
reviewed
Dec 15, 2025
Comment thread
src/collectors/windows.plugin/GetSensors.c
stelfrag
approved these changes
Dec 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR proceeds with the fixes stated in #21454
Test Plan
Additional Information
For users: How does this change affect me?Summary by cubic
Fixes incorrect perflib mappings, labels, and priorities across the Windows plugin so charts show the right counters, units, and names. Also hardens driver install path handling and sensor collection thread safety/cleanup.
Written for commit 877b787. Summary will update automatically on new commits.