in_windows_exporter_metrics: Add description for performance metrics#2610
in_windows_exporter_metrics: Add description for performance metrics#2610cosmo0920 wants to merge 1 commit into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Windows exporter metrics documentation now covers ChangesWindows exporter metrics documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (2)
pipeline/inputs/windows-exporter-metrics.md (2)
62-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix table column alignment and use lowercase key name in parameter table.
The table pipes are misaligned with the header (markdownlint MD060). Additionally, per the documented convention, parameter tables should use canonical lowercase key names. The table entry
PerformanceCountershould beperformancecounterto match the convention where classic config examples use Title_Case and YAML uses snake_case.-| `collector.performancecounter.scrape_interval` | The rate in seconds at which `performancecounter` metrics are collected. Values greater than `0` override the global default. Otherwise, the global default is used. | `0` | -| `PerformanceCounter` | Define a Windows Performance Counter to collect. This key can be specified multiple times. Use the `name=counter_path` form. | _none_ | +| `collector.performancecounter.scrape_interval` | The rate in seconds at which `performancecounter` metrics are collected. Values greater than `0` override the global default. Otherwise, the global default is used. | `0` | +| `performancecounter` | Define a Windows Performance Counter to collect. This key can be specified multiple times. Use the `name=counter_path` form. | _none_ |As per coding guidelines, parameter tables should document the canonical lowercase key names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` around lines 62 - 63, The parameter table has a Markdown alignment issue and one key uses the wrong casing. Update the table in windows-exporter-metrics so the pipes align cleanly with the header, and rename the `PerformanceCounter` entry to the canonical lowercase `performancecounter` to match the documented parameter naming convention.Source: Learnings
89-89: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix table column alignment.
The table pipe alignment is off (markdownlint MD060). Adjust spacing to match the header column widths.
-| `performancecounter` | Exposes user-defined Windows Performance Counters through PDH. | Windows | v5.0.8 | +| `performancecounter` | Exposes user-defined Windows Performance Counters through PDH. | Windows | v5.0.8 |Also consider expanding PDH on first use for clarity: "Performance Data Helper (PDH)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 89, The Windows exporter metrics table row for performancecounter is misaligned with the header, causing markdownlint MD060; update the spacing in that table entry so the pipe columns line up with the existing header widths. While touching this row, expand the PDH acronym on first use in the description by using the full “Performance Data Helper (PDH)” wording for clarity.
🧹 Nitpick comments (2)
pipeline/inputs/windows-exporter-metrics.md (2)
430-430: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse sentence-style capitalization for consistency.
Change to "Custom performance counters" to align with the sentence-style capitalization used in the "Custom scrape intervals per collector" heading (line 392).
-### Custom Performance Counters +### Custom performance countersAs per path instructions, the vale style checker flags this heading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 430, The heading “Custom Performance Counters” uses title-style capitalization and is flagged by the vale style checker; update the heading text in the markdown section to sentence-style capitalization to match “Custom scrape intervals per collector” and the rest of the document’s heading convention.
432-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine PDH on first use or move definition earlier.
PDH is first used at line 89 ("through PDH") and again at line 432, but not defined until line 483 ("Performance Data Helper (PDH)"). Either expand PDH at line 89 or move the definition earlier.
-Exposes user-defined Windows Performance Counters through PDH. +Exposes user-defined Windows Performance Counters through Performance Data Helper (PDH).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 432, The acronym PDH is used before it is defined in the `windows-exporter-metrics` content, so update the earlier `performancecounter` description to expand PDH on first use or move the “Performance Data Helper (PDH)” definition earlier in the document. Make sure the first occurrences around the `performancecounter` collector section and the later PDH definition stay consistent and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@pipeline/inputs/windows-exporter-metrics.md`:
- Around line 62-63: The parameter table has a Markdown alignment issue and one
key uses the wrong casing. Update the table in windows-exporter-metrics so the
pipes align cleanly with the header, and rename the `PerformanceCounter` entry
to the canonical lowercase `performancecounter` to match the documented
parameter naming convention.
- Line 89: The Windows exporter metrics table row for performancecounter is
misaligned with the header, causing markdownlint MD060; update the spacing in
that table entry so the pipe columns line up with the existing header widths.
While touching this row, expand the PDH acronym on first use in the description
by using the full “Performance Data Helper (PDH)” wording for clarity.
---
Nitpick comments:
In `@pipeline/inputs/windows-exporter-metrics.md`:
- Line 430: The heading “Custom Performance Counters” uses title-style
capitalization and is flagged by the vale style checker; update the heading text
in the markdown section to sentence-style capitalization to match “Custom scrape
intervals per collector” and the rest of the document’s heading convention.
- Line 432: The acronym PDH is used before it is defined in the
`windows-exporter-metrics` content, so update the earlier `performancecounter`
description to expand PDH on first use or move the “Performance Data Helper
(PDH)” definition earlier in the document. Make sure the first occurrences
around the `performancecounter` collector section and the later PDH definition
stay consistent and readable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41c7ab5e-44bd-41a5-ab4a-db0291a4c8df
📒 Files selected for processing (1)
pipeline/inputs/windows-exporter-metrics.md
eschabell
added
waiting-on-code-merge
waiting-on-review
eschabell
force-pushed
the
cosmo0920-add-docs-for-user-defined-performance-counters
branch
from
870296a to
996a855
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pipeline/inputs/windows-exporter-metrics.md (3)
63-63: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse canonical lowercase key name and clarify metric name placeholder.
Per the repository convention, parameter tables should document canonical lowercase key names. The table currently shows
PerformanceCounter(Title_Case); it should beperformancecounter.Additionally, the description says
name=counter_path, but the example and surrounding text usemetric_name. Clarify that the metric name is user-defined, or align the description withmetric_name.📝 Proposed fix
-| `PerformanceCounter` | Define a Windows Performance Counter to collect. This key can be specified multiple times. Use the `name=counter_path` form. | _none_ | +| `performancecounter` | Define a Windows Performance Counter to collect. This key can be specified multiple times. Use the `<metric_name>=<counter_path>` form. | _none_ |Based on learnings: "Parameter tables should document the canonical lowercase key names."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 63, The parameter table entry in windows-exporter-metrics.md uses a non-canonical key name and inconsistent placeholder text. Update the PerformanceCounter row to use the canonical lowercase key name in the left column, and align the description with the surrounding metric naming convention by replacing the ambiguous name=counter_path wording with metric_name or a clear note that the metric name is user-defined.Source: Learnings
89-89: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winExpand PDH on first use.
"PDH" appears at line 89 without expansion. The first expansion of "Performance Data Helper (PDH)" occurs much later at line 483. Readers encountering this acronym in the collectors table will not know what it means.
📝 Proposed fix
-| `performancecounter` | Exposes user-defined Windows Performance Counters through PDH. | Windows | v5.0.8 | +| `performancecounter` | Exposes user-defined Windows Performance Counters through Performance Data Helper (PDH). | Windows | v5.0.8 |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 89, The collectors table entry for performancecounter uses the acronym PDH before it is defined, so update that description in windows-exporter-metrics.md to spell out Performance Data Helper (PDH) on first mention. Keep the later full explanation consistent, and ensure the collectors table remains readable without requiring readers to find the later expansion.
430-430: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse sentence-style capitalization for heading.
Per the documentation style guide, headings should use sentence-style capitalization: "Custom performance counters".
📝 Proposed fix
-### Custom Performance Counters +### Custom performance counters🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/inputs/windows-exporter-metrics.md` at line 430, The heading in the documentation uses title case instead of sentence-style capitalization. Update the “Custom Performance Counters” heading in the markdown content to match the style guide by using “Custom performance counters”; keep the change localized to that heading so the surrounding documentation stays unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pipeline/inputs/windows-exporter-metrics.md`:
- Line 43: Fix the wording in the `we.service.include` documentation entry so
the description uses “key-value pairs” instead of “key value pairs.” Update the
markdown table text in the windows exporter metrics docs and keep the rest of
the sentence unchanged.
---
Nitpick comments:
In `@pipeline/inputs/windows-exporter-metrics.md`:
- Line 63: The parameter table entry in windows-exporter-metrics.md uses a
non-canonical key name and inconsistent placeholder text. Update the
PerformanceCounter row to use the canonical lowercase key name in the left
column, and align the description with the surrounding metric naming convention
by replacing the ambiguous name=counter_path wording with metric_name or a clear
note that the metric name is user-defined.
- Line 89: The collectors table entry for performancecounter uses the acronym
PDH before it is defined, so update that description in
windows-exporter-metrics.md to spell out Performance Data Helper (PDH) on first
mention. Keep the later full explanation consistent, and ensure the collectors
table remains readable without requiring readers to find the later expansion.
- Line 430: The heading in the documentation uses title case instead of
sentence-style capitalization. Update the “Custom Performance Counters” heading
in the markdown content to match the style guide by using “Custom performance
counters”; keep the change localized to that heading so the surrounding
documentation stays unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aac7f365-2e59-4b38-b61c-e5610b94fdea
📒 Files selected for processing (1)
pipeline/inputs/windows-exporter-metrics.md
eschabell
force-pushed
the
cosmo0920-add-docs-for-user-defined-performance-counters
branch
from
996a855 to
ae7add1
Compare
|
Fixing a few listing and ai issues. |
… performance metrics - Fixed linting errors. Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io> Signed-off-by: Eric D. Schabell <eric@schabell.org>
eschabell
force-pushed
the
cosmo0920-add-docs-for-user-defined-performance-counters
branch
from
ae7add1 to
a5445db
Compare
eschabell
left a comment
eschabell
left a comment
There was a problem hiding this comment.
@cosmo0920 fixed some listing and other ai uncovered issue, but now good to go when code PR merges.
2 participants
The corresponding PR is:
fluent/fluent-bit#12006
Summary by CodeRabbit
performancecountercollector, including it in the “Collectors available” list.collector.performancecounter.scrape_intervaloverrides and repeatablePerformanceCountermappings usingname=counter_path.windows_performancecounter_<metric_name>), theinstancelabel, and wildcard expansion of counter paths at startup.