Conversation
…er profile DCGM 4.7.0 no longer supports DCGM_FI_DEV_PCIE_RX_THROUGHPUT (201) and DCGM_FI_DEV_PCIE_TX_THROUGHPUT. dcgm-exporter 4.8.4 refuses to start with a counters file that lists them: failed to get counters: DCGM field "DCGM_FI_DEV_PCIE_RX_THROUGHPUT" (ID 201) is no longer supported; use "DCGM_FI_PROF_PCIE_RX_BYTES" instead The replacements DCGM_FI_PROF_PCIE_RX_BYTES and DCGM_FI_PROF_PCIE_TX_BYTES are already enabled in the profile, so the two obsolete lines are removed and the enabled-field count goes from 123 to 121 in the header, the test and the docs.
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe DCGM exporter field count changed from 123 to 121 in the semantics test and related documentation. ChangesDCGM exporter field-count alignment
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (2 skipped: 2 unsupported.)
✨ 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 |
PR Summary by Qododcgm: remove unsupported PCIe throughput fields from exporter profile
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Generated documentation can drift
|
| (raw download: `https://raw.githubusercontent.com/netdata/netdata/master/src/go/plugin/go.d/collector/dcgm/dcgm-exporter-netdata.csv`). | ||
|
|
||
| The Netdata profile enables 123 fields by default and lists optional fields from its source dataset as commented entries. | ||
| The Netdata profile enables 121 fields by default and lists optional fields from its source dataset as commented entries. |
There was a problem hiding this comment.
1. Generated documentation can drift 📜 Skill insight ⌂ Architecture
The generated integration page is edited directly to change the enabled-field count instead of being regenerated from metadata.yaml. Future metadata regeneration can overwrite this manual change, leaving the published page inconsistent with the profile and its source metadata.
Agent Prompt
## Issue description
The integration page is generated from `metadata.yaml` and must not be edited directly. The changed enabled-field count should be produced by the repository's metadata generation workflow.
## Fix Focus Areas
- src/go/plugin/go.d/collector/dcgm/integrations/nvidia_data_center_gpu_manager_dcgm.md[88-88]
- src/go/plugin/go.d/collector/dcgm/metadata.yaml[60-60]
## Recommended Fix
Keep the count change in `metadata.yaml`, remove the direct edit from the generated integration page, and regenerate the page using the repository's documented metadata generation command before committing the generated output if required by the delivery workflow.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 5/5
- In
src/go/plugin/go.d/collector/dcgm/dcgm-exporter-netdata.csv, stale RX/TX throughput annotation comments remain after the related entries changed, which could leave misleading metadata for maintainers or consumers; remove or update the orphaned comments.
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/go/plugin/go.d/collector/dcgm/dcgm-exporter-netdata.csv">
<violation number="1" location="src/go/plugin/go.d/collector/dcgm/dcgm-exporter-netdata.csv:10">
P3: The two `# numeric=dcgm.gpu.raw.dcgm_fi_dev_pcie_rx_throughput` / `..._tx_throughput (raw; physical unit not established)` annotation comments (current lines 908-909) were left behind when their `DCGM_FI_DEV_PCIE_RX/TX_THROUGHPUT` field lines were deleted. They are the only annotations in the file that do not sit directly above a `DCGM_FI_*` entry (enabled or commented); every other `# numeric=`/`# context=`/`# metadata=` annotation documents the field on the following line. With no field line beneath them they now describe entries that do not exist in the profile, which contradicts the header's "uncomment one line" guidance. Remove the two orphan annotation lines together with the removed fields.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Profile as dcgm-exporter-netdata.csv
participant Exporter as dcgm-exporter daemon on host
participant DCGM as NVIDIA DCGM library in driver
participant Coll as go.d dcgm collector plugin
participant Agent as Netdata Agent metrics database
participant CI as dcgm collector unit tests
Note over Profile,Agent: Runtime data flow (current state)
Exporter->>Profile: Load enabled field list at startup
Profile-->>Exporter: 121 enabled fields (PCIe via DCGM_FI_PROF_PCIE_RX_BYTES / TX_BYTES)
Exporter->>DCGM: Query enabled fields through DCGM API
alt All requested fields supported
DCGM-->>Exporter: Per-GPU metric values
Exporter->>Exporter: Serve /metrics endpoint (Prometheus text format)
Coll->>Exporter: Scrape /metrics over HTTP
Exporter-->>Coll: GPU metrics (counters, gauges, profiling bytes)
Coll->>Agent: Submit collected chart data
else Field not supported by DCGM 4.7+
DCGM-->>Exporter: Error: field is no longer supported
Exporter->>Exporter: Abort startup, no /metrics endpoint
end
Note over CI,Profile: Validation path (TestExporterCSVSemantics)
CI->>Profile: Parse CSV rows and count enabled fields
CI->>CI: Compare enabled count to expected stock baseline
alt 121 enabled fields
CI-->>CI: Test passes
else Count mismatch
CI-->>CI: Test fails (preserve stock field selection)
end
Note over CI,Profile: metadata.yaml and the integration page state the same 121-field baseline
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -7,8 +7,8 @@ | |||
| # Example: | |||
There was a problem hiding this comment.
P3: The two # numeric=dcgm.gpu.raw.dcgm_fi_dev_pcie_rx_throughput / ..._tx_throughput (raw; physical unit not established) annotation comments (current lines 908-909) were left behind when their DCGM_FI_DEV_PCIE_RX/TX_THROUGHPUT field lines were deleted. They are the only annotations in the file that do not sit directly above a DCGM_FI_* entry (enabled or commented); every other # numeric=/# context=/# metadata= annotation documents the field on the following line. With no field line beneath them they now describe entries that do not exist in the profile, which contradicts the header's "uncomment one line" guidance. Remove the two orphan annotation lines together with the removed fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/go/plugin/go.d/collector/dcgm/dcgm-exporter-netdata.csv, line 10:
<comment>The two `# numeric=dcgm.gpu.raw.dcgm_fi_dev_pcie_rx_throughput` / `..._tx_throughput (raw; physical unit not established)` annotation comments (current lines 908-909) were left behind when their `DCGM_FI_DEV_PCIE_RX/TX_THROUGHPUT` field lines were deleted. They are the only annotations in the file that do not sit directly above a `DCGM_FI_*` entry (enabled or commented); every other `# numeric=`/`# context=`/`# metadata=` annotation documents the field on the following line. With no field line beneath them they now describe entries that do not exist in the profile, which contradicts the header's "uncomment one line" guidance. Remove the two orphan annotation lines together with the removed fields.</comment>
<file context>
@@ -7,8 +7,8 @@
#
-# This profile contains 623 fields from its original DCGM source dataset.
-# Exactly 123 fields are enabled by default below (127 max per field group).
+# This profile contains 621 fields from its original DCGM source dataset.
+# Exactly 121 fields are enabled by default below (127 max per field group).
# To enable a different field, uncomment one line and comment another enabled line.
</file context>
|
Summary
DCGM 4.7.0 removed
DCGM_FI_DEV_PCIE_RX_THROUGHPUT(ID 201) andDCGM_FI_DEV_PCIE_TX_THROUGHPUT. With those two fields listed,dcgm-exporter4.8.4 exits at startup:The Netdata profile (
dcgm-exporter-netdata.csv) already enables the suggested replacementsDCGM_FI_PROF_PCIE_RX_BYTESandDCGM_FI_PROF_PCIE_TX_BYTES, so this PR removes the two obsolete lines and updates the enabled-field count (123 to 121) in the profile header,TestExporterCSVSemantics,metadata.yamland the generated integration page.Test Plan
go test ./plugin/go.d/collector/dcgm/passes./metricsservesDCGM_FI_PROF_PCIE_RX_BYTES/DCGM_FI_PROF_PCIE_TX_BYTESfor both GPUs.Additional Information
Older DCGM releases still accept the removed fields, but the profiling replacements were already the enabled way to get PCIe throughput in this profile, so nothing is lost on those versions either.
For users: How does this change affect me?
Users of the DCGM collector who run the recommended
dcgm-exporter-netdata.csvprofile with DCGM 4.7 or newer: the exporter now starts instead of exiting on the removed fields. PCIe throughput continues to be collected from the profiling fields.Summary by cubic
Drops two PCIe throughput fields that DCGM 4.7 removed from the
dcgm-exporterprofile so the exporter starts cleanly instead of exiting at startup.DCGM_FI_PROF_PCIE_RX_BYTESandDCGM_FI_PROF_PCIE_TX_BYTES, so PCIe throughput is still collected on both DCGM 4.7 and older releases.metadata.yaml, and integration docs.Written for commit ddaf5c7. Summary will update on new commits.
Summary by CodeRabbit