feat(snmp): improve partial collection resilience and dependency caching - #1489
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the SNMP input’s resilience when collecting partially-available data by introducing a configurable “partial” error policy, dependency-aware row gating, and a per-agent dependency cache. It also adds a new per-agent runtime layer that tracks request/response evidence for health decisions and emits richer collection/build metrics.
Changes:
- Add
error_policysupport (legacyvspartial) for SNMP tables, including dependency-aware row gating and structured build stats. - Introduce per-agent runtime state (health tracking, request/response evidence, recovery probes) and a dependency cache with TTL/capacity controls.
- Expand test coverage significantly to validate partial-mode behavior, caching semantics, and runtime health transitions/metrics.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| inputs/snmp/wrapper.go | Adds connection Close() and hooks to record SNMP send/recv/finish events for runtime health stats. |
| inputs/snmp/table.go | Implements partial error policy, dependency planning/caching, row gating, and build statistics. |
| inputs/snmp/table_test.go | Adds extensive tests covering partial behavior, cache usage, dependency gating, and runtime stats. |
| inputs/snmp/runtime.go | Introduces agentRuntime and runtimeConnection for health tracking, operation stats, and metric counters. |
| inputs/snmp/instances.go | Wires runtime/caching/error policy into Gather flow and adds build/collection metrics emission. |
| inputs/snmp/health_check.go | Reworks health monitoring into Gather-driven recovery probing with runtime integration. |
| inputs/snmp/dependency_cache.go | Adds dependency cache implementation with TTL, capacity enforcement, and stats. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
inputs/snmp/instances.go:167
- Mappings lookup uses the normalized agent string (adds "udp://" when the scheme is omitted), so mappings keyed by the original config value (e.g. "127.0.0.1:161") won't be applied to the up/icmp_* metrics even though the rest of Gather uses the raw agent key. Consider looking up mappings by the raw agent first and falling back to the normalized form to keep tag behavior consistent across metrics.
if m, ok := ins.Mappings[target]; ok {
inputs/snmp/runtime.go:444
- agentRuntime.close() detaches and closes the cached connection before waiting for in-flight requests to complete. If Drop() races with an ongoing Walk/Get, this can close the underlying socket while it's still in use, violating the package's "no concurrent use" constraint for a connection and potentially causing data races/undefined behavior. Prefer waiting for outstanding requests before closing cached/probe connections.
conn := rt.detachConnection()
if conn != nil {
_ = conn.Close()
}
No description provided.