PMM-15362 Disable default-on node_exporter collectors

[theTibi]

Summary

• Bug: pmm-admin inventory change agent node-exporter <id> --disable-collectors=<name> reports success and stores the value, but for any collector node_exporter enables by default (cpu, diskstats, meminfo, filesystem, netdev, stat, …) the metrics keep being exported. Disabling a non-default collector (processes, buddyinfo, meminfo_numa) already worked.
• Root cause: nodeExporterConfig only called collectors.FilterOutCollectors("--collector.", …), which strips the redundant enable flag PMM adds. For a collector node_exporter turns on by itself that is a no-op, so it kept running. The sibling exporters already solve this — postgresql.go and mongodb.go both follow FilterOutCollectors with collectors.DisableDefaultEnabledCollectors; node.go was simply missing that second call.
• Fix: emit --no-collector.<name> for the default-on collectors PMM enables, via the same helper.

Why the list is a whitelist, and deliberately not "every default-on collector"

An unknown --no-collector.<name> makes node_exporter exit at startup, which would take all metrics for that host — far worse than the bug. So defaultNodeExporterCollectors contains only collectors nodeExporterConfig already passes --collector.<name> for. That is self-proving: if the positive flag is already accepted by every agent we drive today, the negation is too. No version gate is needed, unlike postgresql.go, whose collectors are not passed positively.

os was considered and excluded: it is default-on and does appear in PMM's LR collect[] list, but the collector only exists from node_exporter 1.3.0 (CHANGELOG.md, 2021-10-20) while this code still builds configs for agentVersion 2.15.1 — an unprovable risk with total-metric-loss failure mode, for ~2 series VictoriaMetrics already stops ingesting.

Two related fixes in the same blast radius

• DisableDefaultEnabledCollectors now collapses repeated names. Without it, --disable-collectors=cpu,cpu produces two --no-collector.cpu args and node_exporter refuses to start (flag 'collector.cpu' cannot be repeated) — verified live, see below. This also protects the mongodb and postgres call sites.
• inventory change agent node-exporter now runs --disable-collectors through commands.ParseDisableCollectors, as the add path already does. Previously --disable-collectors="cpu, meminfo" stored " meminfo" untrimmed, matching nothing — the ticket's "accepted but does nothing" symptom via the ticket's own command. The reported change now shows the parsed value instead of the raw one.

Ticket

• PMM-15362

Test plan

Unit:

• go test ./managed/services/agents/ -run TestNodeExporterConfig — LinuxDisabledCollectors fails on the base commit with exactly the 5 missing --no-collector.* flags, passes here. New subtests: LinuxDisabledCollectorsNotDefaultEnabled, LinuxAllDefaultCollectorsDisabled (also pins that every entry has a matching --collector.<name>, and that none collides with the static --no-collector.* block), MacOSDisabledCollectors.
• go test ./managed/utils/collectors/... — new duplicate-input case.
• go test ./admin/commands/... — new DisableCollectorsAreTrimmed case; fails against the pre-fix file.
• Each new test mutation-verified (revert the code → the test fails).
• make init + bin/golangci-lint run --new-from-rev=origin/main → 0 issues; gofumpt/goimports/gci clean; go-sumtype clean; check-license 0 invalid; go build ./... clean.

Live, on an isolated PMM 3.10.0 server whose stock pmm-managed is built from this PR's base commit, with a registered pmm-agent (node_exporter 1.8.2) — only the pmm-managed/pmm-admin binaries swapped:

• Before — node_disk_ = 324; run the ticket's command; API stores ["diskstats"]; --collector.diskstats gone, no --no-collector.diskstats; node_disk_ = 324. Bug reproduced end to end.
• After — --no-collector.diskstats present on the real exporter cmdline; node_disk_ = 0; node_cpu_seconds_total = 88 unchanged.
• Duplicate input --disable-collectors=diskstats,diskstats,cpu,diskstats — exactly one flag each, exporter Running, both families 0.
• Counterfactual — same input against a build with this fix but without the dedupe: exporter status Done, agent log repeating node_exporter: error: flag 'collector.diskstats' cannot be repeated, listen port walking 42000→42025. The dedupe is load-bearing.
• Trim — --disable-collectors="cpu, meminfo" stores ['cpu','meminfo'], both --no-collector.* flags present, both families 0.
• Reversibility — replacing the disabled list brought node_disk_ back to 324.
• Non-default control — --disable-collectors=processes: enable flag removed, no --no-collector.processes added, node_processes_ = 0, node_disk_ still 324.
• Flag acceptance on node_exporter 1.4.0 (percona/pmm-client:2) and 1.8.2 (percona/pmm-client:3); a bogus name is correctly rejected on both, confirming the check discriminates.

Not covered: darwin (no macOS node — unit + mutation test only); VictoriaMetrics/Grafana ingestion; a live matrix of older agents (1.4.0 exercised as a standalone binary only).

• pmm-submodules (FB) PR: PMM-15362 Disable default-on node_exporter collectors Percona-Lab/pmm-submodules#4547

Known residuals, deliberately out of scope

1. Roughly 17 default-on collectors PMM neither enables nor disables (pressure, schedstat, softnet, os, dmi, udp_queues, nvme, btrfs, cpufreq, selinux, thermal_zone, watchdog, rapl, fibrechannel, powersupplyclass, tapestats, textfile) remain silent no-ops for --disable-collectors. Only os is scraped by PMM; the rest never reach VictoriaMetrics, so the cost is host-side CPU. Handling them safely needs per-collector version gating.
2. Between the exporter restart and the VictoriaMetrics config regenerating, a stale collect[]=<name> makes node_exporter answer HTTP 400 for the whole scrape job (~1–2 HR scrapes; none in push mode). This is pre-existing, not introduced here: verified live that on the base commit --disable-collectors=processes produces the identical 400 disabled collector: processes, because FilterOutCollectors disables the collector while the scrape config still names it. Worth knowing so the 400 in an exporter log is not mistaken for a new bug.
3. Five sibling change agent commands (mysqld, mongodb, postgres, proxysql, valkey) still pass --disable-collectors untrimmed — PMM-14919's scope.

Note for the release note

Because collect[] is an exclusive filter and the scrape config already drops a disabled collector, the metrics had already stopped reaching Grafana before this fix. The real gains are that the collector stops executing on the monitored host, a direct /metrics scrape no longer exposes it, and the empty-collect[] fallback no longer resurrects it.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.01%. Comparing base (31318c7) to head (a659ad6).
⚠️ Report is 150 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5839      +/-   ##
==========================================
+ Coverage   43.59%   44.01%   +0.41%     
==========================================
  Files         415      302     -113     
  Lines       43134    32883   -10251     
==========================================
- Hits        18804    14473    -4331     
+ Misses      22454    16896    -5558     
+ Partials     1876     1514     -362     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d5e06760-c82c-42b2-98dd-93738d7fb6b6

📥 Commits

Reviewing files that changed from the base of the PR and between 6729e04 and a659ad6.

📒 Files selected for processing (6)

• admin/commands/inventory/change_agent_node_exporter.go
• admin/commands/inventory/change_agent_node_exporter_test.go
• managed/services/agents/node.go
• managed/services/agents/node_test.go
• managed/utils/collectors/collectors.go
• managed/utils/collectors/collectors_test.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• percona/pmm-qa (manual)
• percona/pmm (manual)

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

Arrr, the change normalizes disabled collector input, removes duplicate disable flags, and applies default collector disabling on non-macOS Node Exporter configurations.

Changes

Node Exporter collector configuration

Layer / File(s)	Summary
Normalize disabled collector input admin/commands/inventory/change_agent_node_exporter.go, admin/commands/inventory/change_agent_node_exporter_test.go	The command parses and trims disabled collectors before API submission and change reporting.
Deduplicate default collector flags managed/utils/collectors/collectors.go, managed/utils/collectors/collectors_test.go	The collector utility emits unique --no-collector arguments in first-occurrence order.
Apply platform-specific Node Exporter flags managed/services/agents/node.go, managed/services/agents/node_test.go	Non-macOS configurations add disable flags for disabled default collectors. Tests cover Linux, non-default collectors, all default collectors, and macOS behavior.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: disabling default-on node_exporter collectors.
Description check	✅ Passed	The description thoroughly explains the bug, root cause, fix, scope, testing, and related work. It omits the template's Feature build field, but the required change details are otherwise complete.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}