You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(go.d/wireguard): remove only the removed device's charts - #24568
Follow-up to #24542 (same bug class, refs #24230).
wireguardremoveDeviceCharts matches chart IDs by the unterminated prefix device_<name>, so removing wg1 also marks every chart of wg10-wg19 obsolete. The surviving device stays in c.devices, so its charts are never added again until go.d.plugin restarts.
It now matches the exact chart IDs built from deviceChartsTmpl, like ntpd and nvme in #24542. Peer chart IDs contain the full 44-character public key, so removePeerCharts can't hit another peer this way and is left alone.
Test Plan
New TestCollector_Collect case "device removed at run time, device with a longer name is kept": it collects wg1 and wg10, drops wg1, runs the cleanup, and checks that only the device_wg1_* charts are obsolete. On master it fails (device_wg10_network_io and device_wg10_peers are obsolete too); with the fix it passes. go test -count=1 ./plugin/go.d/collector/wireguard/ and go vet pass.
Additional Information
I used Claude Code to write this change.
For users: How does this change affect me?
WireGuard (go.d/wireguard) device charts.
When a WireGuard interface goes away, the charts of other interfaces whose name starts with the same string (e.g. wg10 after wg1) no longer disappear.
Summary by cubic
Fixes the WireGuard collector so removing a device only removes that device's charts, not the charts of devices with longer names that share the same prefix (e.g., removing wg1 no longer marks wg10 charts as obsolete).
Matches exact chart IDs from deviceChartsTmpl instead of using an unterminated prefix match.
Adds a test case covering device removal with a longer-named device still present.
Written for commit 0471c56. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Removing a WireGuard device now marks only that device’s charts as obsolete. Charts for other devices with similar names remain available, preventing unintended chart cleanup when device names share a prefix.
removeDeviceCharts matched chart IDs by an unterminated prefix,
"device_<name>", so removing wg1 also marked every chart of wg10-wg19
as obsolete. The surviving device stays in c.devices, so its charts
were never re-created until the plugin restarted.
Match the exact chart IDs built from deviceChartsTmpl instead, as in
ntpd and nvme.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
No actionable comments were generated in the recent review.
Walkthrough
WireGuard device chart cleanup now matches exact chart IDs generated from the device chart template. A collection test covers removing wg1 while retaining wg10.
removeDeviceCharts matches exact IDs generated from deviceChartsTmpl. The collection test verifies that cleanup marks wg1 charts obsolete without marking wg10 charts obsolete.
The PR narrowly fixes device chart cleanup and adds coverage for shared device-name prefixes; no merge-blocking risk remains.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: removing only charts for the removed WireGuard device.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
Confidence score: 5/5
Automated review surfaced no issues in the provided summaries.
No files require special attention.
Architecture diagram
sequenceDiagram
participant C as Collector
participant D as Device Manager
participant Ch as Chart Store
participant T as Chart Templates
Note over C,T: WireGuard Chart Cleanup Flow
C->>D: Collect() - fetch devices
D-->>C: device list (wg1, wg10)
loop Each device
C->>C: Track active devices
end
alt Device wg1 removed at runtime
C->>C: Detect wg1 no longer in device list
C->>C: Call removeDeviceCharts("wg1")
Note over C,T: NEW: Build exact chart ID set
C->>T: Read deviceChartsTmpl definitions
T-->>C: Chart templates (network_io, peers, etc.)
loop Each chart template
C->>C: Generate exact chart ID (e.g., device_wg1_network_io)
C->>C: Add to lookup set
end
C->>Ch: Iterate all existing charts
alt Chart ID matches exact set
C->>Ch: MarkRemove() + MarkNotCreated()
Note over Ch: Only charts for wg1 affected
else Chart ID doesn't match
Note over Ch: Keep charts (e.g., device_wg10_*)
end
end
opt Device wg10 still active
C->>Ch: Keep wg10 charts in registry
Note over Ch: Charts survive cleanup
end
Makes sense, thanks. Should I close this one (and #24539, the same thing for docker)? If a v2 conversion of the wireguard collector would be welcome as a PR, I can try that instead.
Yes, you can close all of them. There's a skill in the repo for converting v1 to v2, so Claude should be able to figure it out. Also, ask Claude to remove the AI contribution attribution from commits.
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
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.
github-actions
Bot
added
area/collectors
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary
Follow-up to #24542 (same bug class, refs #24230).
wireguardremoveDeviceChartsmatches chart IDs by the unterminated prefixdevice_<name>, so removingwg1also marks every chart ofwg10-wg19obsolete. The surviving device stays inc.devices, so its charts are never added again until go.d.plugin restarts.It now matches the exact chart IDs built from
deviceChartsTmpl, like ntpd and nvme in #24542. Peer chart IDs contain the full 44-character public key, soremovePeerChartscan't hit another peer this way and is left alone.Test Plan
New
TestCollector_Collectcase "device removed at run time, device with a longer name is kept": it collectswg1andwg10, dropswg1, runs the cleanup, and checks that only thedevice_wg1_*charts are obsolete. On master it fails (device_wg10_network_ioanddevice_wg10_peersare obsolete too); with the fix it passes.go test -count=1 ./plugin/go.d/collector/wireguard/andgo vetpass.Additional Information
I used Claude Code to write this change.
For users: How does this change affect me?
wg10afterwg1) no longer disappear.Summary by cubic
Fixes the WireGuard collector so removing a device only removes that device's charts, not the charts of devices with longer names that share the same prefix (e.g., removing
wg1no longer markswg10charts as obsolete).deviceChartsTmplinstead of using an unterminated prefix match.Written for commit 0471c56. Summary will update on new commits.
Summary by CodeRabbit