Skip to content

Conversation

@ilyam8
Copy link
Member

@ilyam8 ilyam8 commented Dec 5, 2025

Summary
Test Plan
Additional Information
For users: How does this change affect me?

Summary by cubic

Add per-collection stats to the SNMP go.d collector and expose them as per-profile charts. Stats track timing, SNMP ops, cache performance, errors, and metric totals, and are returned in ProfileMetrics.

  • New Features

    • Added CollectionStats to ProfileMetrics with timing, SNMP GET/WALK counts, metric counts (scalar/table/virtual, tables, rows), table cache hits/misses, and error counters; exposed as per-profile charts.
    • Stats are populated across scalar, table, and virtual phases, including tables walked/cached and rows processed.
    • Logs when expired table cache entries are cleared.
  • Refactors

    • Converted internal collectors to unexported collect() methods; updated call sites and tests.
    • Instrumented SNMP GET/WALK and cache paths to update stats; error paths increment counters.
    • Added a unit test that validates a stats snapshot for a toy profile.

Written for commit c6d07d8. Summary will update automatically on new commits.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive collection statistics to the SNMP collector to improve observability and debugging capabilities.

  • Introduces CollectionStats structure to track timing, SNMP operations, metric counts, cache performance, and errors for each profile collection cycle
  • Refactors internal collector methods from exported to unexported (Collectcollect) for better encapsulation
  • Threads stats tracking through all collection phases (scalar, table, virtual metrics)

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/go/plugin/go.d/collector/snmp/ddsnmp/metric.go Adds CollectionStats and related stats types (TimingStats, SNMPOperationStats, MetricCountStats, TableCacheStats, ErrorStats) with comprehensive documentation
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector.go Integrates stats collection, refactors profileState to use nested cache struct, measures timing for each collection phase
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_scalar.go Converts Collect to unexported collect(), adds stats parameter, tracks SNMP GET operations, missing OIDs, and processing errors
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table.go Converts Collect to unexported collect(), tracks SNMP walks, cache hits/misses, tables walked/cached, row counts, and errors throughout the collection pipeline
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_vmetrics.go Converts Collect to unexported collect(), removes unnecessary line break in function signature
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_global_tags.go Converts Collect to unexported collect() for consistency
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_device_meta.go Converts Collect to unexported collect() for consistency
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_test.go Adds comprehensive test validating stats snapshot for a simple profile with scalar, table, and virtual metrics
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_scalar_test.go Updates test to use new collect() signature with stats parameter
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table_test.go Updates test to use new collect() signature with stats parameter
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_vmetrics_test.go Updates test to use new collect() method name
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_global_tags_test.go Updates test to use new collect() method name
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_device_meta_test.go Updates test to use new collect() method name

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ilyam8 ilyam8 requested a review from Copilot December 5, 2025 19:59
@ilyam8 ilyam8 marked this pull request as ready for review December 5, 2025 19:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 17 files

@ilyam8 ilyam8 merged commit d763e6f into netdata:master Dec 5, 2025
116 checks passed
@ilyam8 ilyam8 deleted the go.d-snmp-stats branch December 5, 2025 20:23
@stelfrag stelfrag mentioned this pull request Dec 6, 2025
stelfrag pushed a commit to stelfrag/netdata that referenced this pull request Dec 6, 2025
Ferroin pushed a commit that referenced this pull request Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/collectors Everything related to data collection area/go collectors/go.d

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants