Updates for Windows system metrics #118
Conversation
Summary by CodeRabbit
WalkthroughDocumentation update to src/pages/integrations/windows.mdx: shifts InstallIntegration to metrics-only usage and substantially revises the Telegraf Windows performance counters sample config. Removes win_services, expands CPU/process/disk/network/system/memory/paging metrics, adds PhysicalDisk and Network Interface objects, and moves outputs.http to a top-level block. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
added
the
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
src/pages/integrations/windows.mdx (3)
100-106: Update text: sample config no longer includes Windows Services.The paragraph says the config “includes plugins for both Windows Performance Counters and Windows Services,” but there’s no [[inputs.win_services]] block below. Adjust the copy to avoid confusing users.
Apply:
- It includes plugins for both Windows Performance Counters and Windows Services and specifies the output configuration - for sending your data to Logit.io. + It includes plugins for Windows Performance Counters and specifies the output configuration + for sending your data to Logit.io.
254-256: Remove or reframe the Windows Services callout.Since the sample config omits [[inputs.win_services]], linking to that plugin here implies it’s used. Either remove the callout or add a dedicated section with a minimal win_services example if still relevant.
Example to remove:
- <Callout type="info"> - Read more about how to configure data scraping and configuration options for Windows Services [here](https://github.com/influxdata/telegraf/tree/master/plugins/inputs/win_services) - </Callout>Or add a small win_services block if the dashboard includes service status panels.
268-270: Fix quoting for the --config path in PowerShell test command.Mixed quoting (C:"Program Files"...) is error-prone. Use a single quoted or double-quoted full path.
- .\telegraf.exe ` - --config C:\"Program Files"\InfluxData\telegraf\telegraf.conf --test + .\telegraf.exe ` + --config "C:\Program Files\InfluxData\telegraf\telegraf.conf" ` + --test
🧹 Nitpick comments (5)
src/pages/integrations/windows.mdx (5)
115-127: Clarify intent for CPU instances vs IncludeTotal.With Instances = ["_Total"], setting IncludeTotal = true is redundant. If you want per-core + total for dashboards, set Instances = ["*"] and IncludeTotal = true. If the dashboard expects only _Total, keep Instances = ["_Total"] and drop IncludeTotal.
Option A (total only):
Measurement = "win_cpu" - # Set to true to include _Total instance when querying for all (). - IncludeTotal=trueOption B (per-core + total):
- Instances = ["_Total"] + Instances = ["*"] Measurement = "win_cpu" - # Set to true to include _Total instance when querying for all (). - IncludeTotal=true + # Include _Total alongside per-core instances. + IncludeTotal = truePlease confirm which aligns with the Grafana panels in logit-dashboard#968.
128-141: Process instances list and comment mismatch; consider broader capture.Comment says “IIS only” but you enumerate IIS and non-IIS processes (java, httpd, TrendMicro). Hard-coding names is brittle and environment-specific.
- If dashboards are built for system-wide process metrics, prefer Instances = ["*"] and filter/group in Grafana.
- If only IIS worker processes are needed, narrow to Instances = ["w3wp"] and fix the comment.
Example for broad capture:
- # Process metrics, in this case for IIS only + # Process metrics (all processes; filter in dashboard as needed) ObjectName = "Process" @@ - Instances = ["w3wp", "dfmserver", "dfmwatchdog", "java", "httpd", "TrendScan", "TmListen", "CNTAoSMgr"] + Instances = ["*"]Confirm what the updated dashboard expects.
159-171: PhysicalDisk counters look good; consider read/write queue counters if needed.The selected counters are solid for IO throughput and latency. If the dashboard benefits from per-direction queue metrics, you could also include “Avg. Disk Read Queue Length” and “Avg. Disk Write Queue Length.”
Counters = [ "Disk Read Bytes/sec", "Disk Write Bytes/sec", "Avg. Disk Queue Length", "Current Disk Queue Length", "Avg. Disk sec/Read", "Avg. Disk sec/Write", "Split IO/sec", + "Avg. Disk Read Queue Length", + "Avg. Disk Write Queue Length", ]
205-207: Minor: fix misleading comment about IncludeTotal with “------”.The note mentions including _Total “when querying for all (*)” but this block uses Instances = ["------"] (to remove instance dimension). Update the comment to avoid confusion.
- # Set to true to include _Total instance when querying for all (). - #IncludeTotal=false + # Note: Instances = ["------"] removes the instance dimension for this object. + # IncludeTotal is not applicable here. + #IncludeTotal = false
79-86: Consider clarifying the two-step download/extract instruction.The note “press enter after the initial download command to run the extract command” suggests two separate commands; but they’re already separate lines. Optionally reword to “Run the following two commands in sequence” for clarity.
- and then extract that into `C:\Program Files\InfluxData\telegraf\telegraf-1.33.0`. (You will need - to press enter after the initial download command to run the extract command). + and then extract it into `C:\Program Files\InfluxData\telegraf\telegraf-1.33.0`. Run the following two commands in sequence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/integrations/windows.mdx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-04T11:54:28.144Z
Learnt from: pwatson-logit
PR: logit-io/docs-content#116
File: src/pages/integrations/windows.mdx:217-219
Timestamp: 2025-07-04T11:54:28.144Z
Learning: In Telegraf, the service install command uses the syntax `service install` without dashes. The `--service` flag has been deprecated in newer versions of Telegraf.
Applied to files:
src/pages/integrations/windows.mdx
🔇 Additional comments (6)
src/pages/integrations/windows.mdx (6)
71-71: Use of metrics-only InstallIntegration looks correct.Switching to aligns with a metrics-focused path and the PR objective to support the new Grafana dashboard.
195-207: System counters: verify “System Calls/sec” availability on target Windows versions.Some environments don’t expose “System Calls/sec” by default or it may vary by OS/version. Ensure the dashboard and users’ hosts have this counter to avoid empty series.
Would you like me to prepare a quick PowerShell snippet users can run to validate these counters exist on their hosts?
212-226: Memory counters: looks comprehensive.The chosen counters (Available Bytes, Pages/sec, Page Reads/Writes/sec, Standby Cache) cover typical dashboard needs.
262-265: Service installation syntax is correct per current Telegraf guidance.Using “service install” (without deprecated flags) matches our prior learning and current Telegraf behavior.
80-86: Optional: verify Telegraf version.1.33.0 is fine, but if a newer stable (or security) release exists, consider updating to match current support.
Would you like me to check the latest Telegraf Windows release and propose an updated command set if newer?
241-247: Use Basic Auth fields inoutputs.http
File:src/pages/integrations/windows.mdx(lines 241–247)Embedding credentials in the URL can break when passwords contain special characters (e.g.,
@,:,/) and may expose them in logs. Theoutputs.httpplugin supports explicitusername/passwordkeys for Basic Auth.• Replace the embedded credentials in the
urlwith separateusernameandpasswordfields.
• Verify that your MDX templating (the@@metrics_idsyntax, etc.) renders a single@in the final output.Proposed diff:
@@ src/pages/integrations/windows.mdx:241-247 [[outputs.http]] - url = "https://@metricsUsername:@metricsPassword@@metrics_id-vm.logit.io:@vmAgentPort/api/v1/write" + url = "https://@metrics_id-vm.logit.io:@vmAgentPort/api/v1/write" + username = "@metricsUsername" + password = "@metricsPassword" data_format = "prometheusremotewrite" [outputs.http.headers] Content-Type = "application/x-protobuf" Content-Encoding = "snappy"Please confirm that your site build correctly replaces
@metricsUsername,@metricsPassword,@metrics_id, and@vmAgentPort, and that the Basic Auth fields work with your dashboard.
| # Disk times and queues | ||
| ObjectName = "LogicalDisk" | ||
| Instances = ["*"] | ||
| Counters = [ | ||
| "% Idle Time", | ||
| "% Disk Time","% Disk Read Time", | ||
| "% Disk Time", | ||
| "% Disk Read Time", | ||
| "% Disk Write Time", | ||
| "% User Time", | ||
| "Current Disk Queue Length", | ||
| "% Free Space", | ||
| "Free Megabytes", | ||
| ] | ||
| Measurement = "win_disk" | ||
| # Set to true to include _Total instance when querying for all (*). | ||
| #IncludeTotal=false | ||
| # Set to true to include _Total instance when querying for all (). | ||
| #IncludeTotal=true | ||
| [[inputs.win_perf_counters.object]] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate LogicalDisk objects/metrics; consolidate to avoid duplicate series.
LogicalDisk is defined twice, both with Measurement = "win_disk", and the first already includes “% Free Space” and “Free Megabytes”. This will emit duplicate metrics for those counters and can complicate dashboards.
Consolidate into a single LogicalDisk block with IncludeTotal = true, and keep all desired counters. For example:
[[inputs.win_perf_counters.object]]
# Disk times and queues
ObjectName = "LogicalDisk"
- Instances = ["*"]
+ Instances = ["*"]
Counters = [
"% Idle Time",
- "% Disk Time",
- "% Disk Read Time",
+ "% Disk Time",
+ "% Disk Read Time",
"% Disk Write Time",
"Current Disk Queue Length",
"% Free Space",
"Free Megabytes",
]
Measurement = "win_disk"
- # Set to true to include _Total instance when querying for all ().
- #IncludeTotal=true
- [[inputs.win_perf_counters.object]]
- ObjectName = "LogicalDisk"
- Instances = ["*"]
- Counters = [
- "% Free Space",
- "Free Megabytes"
- ]
- Measurement = "win_disk"
- IncludeTotal = true # Optional, but will add an aggregate "_Total" instance
+ # Include aggregate "_Total" instance
+ IncludeTotal = trueAlso applies to: 172-180
🤖 Prompt for AI Agents
In src/pages/integrations/windows.mdx around lines 143-158 (also applies to
172-180), there are two LogicalDisk win_perf_counters.object blocks both using
Measurement = "win_disk" which will produce duplicate series for shared
counters; consolidate them into a single LogicalDisk block by merging the
Counters arrays (keep "% Free Space" and "Free Megabytes" only once), ensure
Instances and Measurement remain correct, and enable IncludeTotal=true
(uncomment or add) if you need the _Total instance, then remove the duplicate
LogicalDisk block so each counter emits only one metric series.
Mike-Logit
left a comment
Mike-Logit
left a comment
There was a problem hiding this comment.
@pwatson-logit does this need testing by you? I'll leave this with you to merge once you are happy
This PR makes some improvements to the Windows system metrics, to work with the new Windows metrics dashboard in Grafana. (See https://github.com/logit-io/logit-dashboard/pull/968).