feat(go.d/snmp): add ping_only option - #22180
Merged
Merged
Conversation
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.
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.
Architecture diagram
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)
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_onlyconfig 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.
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
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.
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
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_onlyto thego.d/snmpcollector. 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
ping_onlyboolean in config/schema; default isfalse.ping_onlyimplies ping is enabled and initializes the prober.ping_onlyis enabled.Check(), ifping_onlyis set, ping is executed and fails only on unrecoverable errors (e.g., EPERM/EACCES); transient errors are allowed.Migration
ping_only: trueto collect only ping RTT metrics.Written for commit e2f7a1e. Summary will update on new commits.