Perflib fixes (Windows.plugin) #21458
Open
+88
−105
Conversation
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
github-actions
bot
added
area/collectors
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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. Improves accuracy and reliability of metrics for IIS, Exchange, .NET, Memory, NUMA, ADFS, Network, Storage, Objects, and Processes.
Written for commit 2c37061. Summary will update automatically on new commits.