feat(go.d/snmp): add ping_only option - #22180
Merged
ilyam8 merged 3 commits intoApr 10, 2026
Merged
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
When enabled, the collector collects only ICMP round-trip time metrics during Collect(). SNMP is still used during initialization to query sysInfo and profile metadata for Virtual Node creation and labeling.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
sequenceDiagram
participant Agent as Netdata Agent
participant Coll as SNMP Collector
participant Device as SNMP Agent (Device)
participant Prober as Ping Prober (ICMP)
Note over Coll,Device: Initialization Phase
Coll->>Device: Get sysInfo (sysDescr, sysObjectID, etc.)
Device-->>Coll: Metadata & Virtual Node Info
alt NEW: ping_only == true OR ping.enabled == true
Coll->>Coll: CHANGED: initProber()
Coll->>Agent: NEW: Create ICMP RTT Charts
end
Note over Agent,Prober: Runtime Collection Loop
Agent->>Coll: Collect()
alt NEW: ping_only == true
Coll->>Prober: Ping(host)
Prober-->>Coll: RTT Statistics (min/max/avg/stddev)
Note right of Coll: Skips all SNMP profile metrics
else standard flow (ping_only == false)
Coll->>Device: SNMP Get/BulkWalk (Profile OIDs)
Device-->>Coll: Device Metrics
opt ping.enabled == true
Coll->>Prober: Ping(host)
Prober-->>Coll: RTT Statistics
end
end
Coll-->>Agent: Return metrics map (map[string]int64)
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a ping_only mode to the go.d/snmp collector to allow collecting only ICMP RTT metrics during Collect(), while still performing SNMP sysInfo/profile initialization needed for labeling/vnode metadata.
Changes:
- Introduces ping_only config flag and exposes it in the JSON schema/UI.
- Updates collector initialization/collection flow to support ping-only collection.
- Adds/updates testdata and unit tests to cover ping-only behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file| File | Description |
|---|---|
| src/go/plugin/go.d/collector/snmp/testdata/config.yaml | Adds ping_only to YAML serialization test fixture. |
| src/go/plugin/go.d/collector/snmp/testdata/config.json | Adds ping_only to JSON serialization test fixture. |
| src/go/plugin/go.d/collector/snmp/config.go | Adds PingOnly to the SNMP collector config struct. |
| src/go/plugin/go.d/collector/snmp/config_schema.json | Adds ping_only to schema + UI schema. |
| src/go/plugin/go.d/collector/snmp/collector.go | Initializes ping prober when ping_only is enabled. |
| src/go/plugin/go.d/collector/snmp/collect.go | Routes collection to ping-only path when enabled; adjusts initialization checks. |
| src/go/plugin/go.d/collector/snmp/collector_test.go | Adds a ping-only collection test and a mock prober. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
src/go/plugin/go.d/collector/snmp/collect.go
Comment thread
src/go/plugin/go.d/collector/snmp/config_schema.json
Comment thread
src/go/plugin/go.d/collector/snmp/config_schema.json
Outdated
Document that ping_only implies ping is enabled regardless of ping.enabled. Fix ui:help to reflect that SNMP sysInfo runs at startup for device identification, not only during vnode setup.
Fail Check() early when ping_only is enabled and ping returns an unrecoverable error (EPERM/EACCES). Transient errors are allowed through since the device may be temporarily unreachable.
thiagoftsm
approved these changes
Apr 10, 2026
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Merged
Ferroin
pushed a commit
to stelfrag/netdata
that referenced
this pull request
Apr 10, 2026
Ferroin
pushed a commit
that referenced
this pull request
Apr 10, 2026
1 task
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
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.
When enabled, the collector collects only ICMP round-trip time metrics during Collect(). SNMP is still used during initialization to query sysInfo and profile metadata for Virtual Node creation and labeling.
Summary
Test Plan
Additional Information
For users: How does this change affect me?Summary by cubic
Adds ping_only to the go.d/snmp collector. When enabled, it collects only ICMP RTT and skips SNMP polling; a minimal SNMP sysInfo runs once at startup for device identification, profile matching, and metadata. Also validates reachability during Check and fails on unrecoverable ping errors.
New Features
Migration
Written for commit e2f7a1e. Summary will update on new commits.