Repository: prometheus/snmp_exporter
Pull request #885 (2023-05-31 10:40:25)

Allow custom registry for initialization of collector metrics
This PR changes the initialization of SNMP internal metrics allowing to specify a custom registry. This is aims to solve the issue described [here](https://github.com/grafana/snmp_exporter/pull/1) but in a generic way. I took a similar approach to this [PR](https://github.com/prometheus/blackbox_exporter/pull/1060) to solve the issue.

marctc: cc @SuperQ 

Patch:
diff --git a/collector/collector.go b/collector/collector.go
index 8056186..ef41c89 100644
--- a/collector/collector.go
+++ b/collector/collector.go
@@ -38,36 +38,6 @@ const (
 )
 
 var (
-	buckets               = prometheus.ExponentialBuckets(0.0001, 2, 15)
-	snmpUnexpectedPduType = promauto.NewCounter(
-		prometheus.CounterOpts{
-			Namespace: namespace,
-			Name:      "unexpected_pdu_type_total",
-			Help:      "Unexpected Go types in a PDU.",
-		},
-	)
-	snmpDuration = promauto.NewHistogram(
-		prometheus.HistogramOpts{
-			Namespace: namespace,
-			Name:      "packet_duration_seconds",
-			Help:      "A histogram of latencies for SNMP packets.",
-			Buckets:   buckets,
-		},
-	)
-	snmpPackets = promauto.NewCounter(
-		prometheus.CounterOpts{
-			Namespace: namespace,
-			Name:      "packets_total",
-			Help:      "Number of SNMP packet sent, including retries.",
-		},
-	)
-	snmpRetries = promauto.NewCounter(
-		prometheus.CounterOpts{
-			Namespace: namespace,
-			Name:      "packet_retries_total",
-			Help:      "Number of SNMP packet retries.",
-		},
-	)
 	// 64-bit float mantissa: https://en.wikipedia.org/wiki/Double-precision_floating-point_format
 	float64Mantissa uint64 = 9007199254740992
 	wrapCounters           = kingpin.Flag("snmp.wrap-large-counters", "Wrap 64-bit counters to avoid floating point rounding.").Default("true").Bool()
@@ -116,7 +86,7 @@ type ScrapeResults struct {
 	retries uint64
 }
 
-func ScrapeTarget(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger) (ScrapeResults, error) {
+func ScrapeTarget(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger, metrics internalMetrics) (ScrapeResults, error) {
 	results := ScrapeResults{}
 	// Set the options.
 	snmp := gosnmp.GoSNMP{}
@@ -136,14 +106,14 @@ func ScrapeTarget(ctx context.Context, target string, auth *config.Auth, module
 	var sent time.Time
 	snmp.OnSent = func(x *gosnmp.GoSNMP) {
 		sent = time.Now()
-		snmpPackets.Inc()
+		metrics.snmpPackets.Inc()
 		results.packets++
 	}
 	snmp.OnRecv = func(x *gosnmp.GoSNMP) {
-		snmpDuration.Observe(time.Since(sent).Seconds())
+		metrics.snmpDuration.Observe(time.Since(sent).Seconds())
 	}
 	snmp.OnRetry = func(x *gosnmp.GoSNMP) {
-		snmpRetries.Inc()
+		metrics.snmpRetries.Inc()
 		results.retries++
 	}
 
@@ -191,7 +161,7 @@ func ScrapeTarget(ctx context.Context, target string, auth *config.Auth, module
 			continue
 		}
 
-		allowedList = filterAllowedIndices(logger, filter, pdus, allowedList)
+		allowedList = filterAllowedIndices(logger, filter, pdus, allowedList, metrics)
 
 		// Update config to get only index and not walk them.
 		newWalk = updateWalkConfig(newWalk, filter, logger)
@@ -272,12 +242,12 @@ func ScrapeTarget(ctx context.Context, target string, auth *config.Auth, module
 	return results, nil
 }
 
-func filterAllowedIndices(logger log.Logger, filter config.DynamicFilter, pdus []gosnmp.SnmpPDU, allowedList []string) []string {
+func filterAllowedIndices(logger log.Logger, filter config.DynamicFilter, pdus []gosnmp.SnmpPDU, allowedList []string, metrics internalMetrics) []string {
 	level.Debug(logger).Log("msg", "Evaluating rule for oid", "oid", filter.Oid)
 	for _, pdu := range pdus {
 		found := false
 		for _, val := range filter.Values {
-			snmpval := pduValueAsString(&pdu, "DisplayString")
+			snmpval := pduValueAsString(&pdu, "DisplayString", metrics)
 			level.Debug(logger).Log("config value", val, "snmp value", snmpval)
 
 			if regexp.MustCompile(val).MatchString(snmpval) {
@@ -366,16 +336,64 @@ func buildMetricTree(metrics []*config.Metric) *MetricNode {
 	return metricTree
 }
 
+type internalMetrics struct {
+	snmpUnexpectedPduType prometheus.Counter
+	snmpDuration          prometheus.Histogram
+	snmpPackets           prometheus.Counter
+	snmpRetries           prometheus.Counter
+}
+
 type collector struct {
-	ctx    context.Context
-	target string
-	auth   *config.Auth
-	module *config.Module
-	logger log.Logger
+	ctx     context.Context
+	target  string
+	auth    *config.Auth
+	module  *config.Module
+	logger  log.Logger
+	metrics internalMetrics
+}
+
+func newInternalMetrics(reg prometheus.Registerer) internalMetrics {
+	buckets := prometheus.ExponentialBuckets(0.0001, 2, 15)
+	snmpUnexpectedPduType := promauto.With(reg).NewCounter(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "unexpected_pdu_type_total",
+			Help:      "Unexpected Go types in a PDU.",
+		},
+	)
+	snmpDuration := promauto.With(reg).NewHistogram(
+		prometheus.HistogramOpts{
+			Namespace: namespace,
+			Name:      "packet_duration_seconds",
+			Help:      "A histogram of latencies for SNMP packets.",
+			Buckets:   buckets,
+		},
+	)
+	snmpPackets := promauto.With(reg).NewCounter(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "packets_total",
+			Help:      "Number of SNMP packet sent, including retries.",
+		},
+	)
+	snmpRetries := promauto.With(reg).NewCounter(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "packet_retries_total",
+			Help:      "Number of SNMP packet retries.",
+		},
+	)
+	return internalMetrics{
+		snmpUnexpectedPduType: snmpUnexpectedPduType,
+		snmpDuration:          snmpDuration,
+		snmpPackets:           snmpPackets,
+		snmpRetries:           snmpRetries,
+	}
 }
 
-func New(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger) *collector {
-	return &collector{ctx: ctx, target: target, auth: auth, module: module, logger: logger}
+func New(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger, reg prometheus.Registerer) *collector {
+	internalMetrics := newInternalMetrics(reg)
+	return &collector{ctx: ctx, target: target, auth: auth, module: module, logger: logger, metrics: internalMetrics}
 }
 
 // Describe implements Prometheus.Collector.
@@ -386,7 +404,7 @@ func (c collector) Describe(ch chan<- *prometheus.Desc) {
 // Collect implements Prometheus.Collector.
 func (c collector) Collect(ch chan<- prometheus.Metric) {
 	start := time.Now()
-	results, err := ScrapeTarget(c.ctx, c.target, c.auth, c.module, c.logger)
+	results, err := ScrapeTarget(c.ctx, c.target, c.auth, c.module, c.logger, c.metrics)
 	if err != nil {
 		level.Info(c.logger).Log("msg", "Error scraping target", "err", err)
 		ch <- prometheus.NewInvalidMetric(prometheus.NewDesc("snmp_error", "Error scraping target", nil, nil), err)
@@ -427,7 +445,7 @@ PduLoop:
 			}
 			if head.metric != nil {
 				// Found a match.
-				samples := pduToSamples(oidList[i+1:], &pdu, head.metric, oidToPdu, c.logger)
+				samples := pduToSamples(oidList[i+1:], &pdu, head.metric, oidToPdu, c.logger, c.metrics)
 				for _, sample := range samples {
 					ch <- sample
 				}
@@ -506,10 +524,10 @@ func parseDateAndTime(pdu *gosnmp.SnmpPDU) (float64, error) {
 	return float64(t.Unix()), nil
 }
 
-func pduToSamples(indexOids []int, pdu *gosnmp.SnmpPDU, metric *config.Metric, oidToPdu map[string]gosnmp.SnmpPDU, logger log.Logger) []prometheus.Metric {
+func pduToSamples(indexOids []int, pdu *gosnmp.SnmpPDU, metric *config.Metric, oidToPdu map[string]gosnmp.SnmpPDU, logger log.Logger, metrics internalMetrics) []prometheus.Metric {
 	var err error
 	// The part of the OID that is the indexes.
-	labels := indexesToLabels(indexOids, metric, oidToPdu)
+	labels := indexesToLabels(indexOids, metric, oidToPdu, metrics)
 
 	value := getPduValue(pdu)
 	t := prometheus.UntypedValue
@@ -568,13 +586,13 @@ func pduToSamples(indexOids []int, pdu *gosnmp.SnmpPDU, metric *config.Metric, o
 		}
 
 		if len(metric.RegexpExtracts) > 0 {
-			return applyRegexExtracts(metric, pduValueAsString(pdu, metricType), labelnames, labelvalues, logger)
+			return applyRegexExtracts(metric, pduValueAsString(pdu, metricType, metrics), labelnames, labelvalues, logger)
 		}
 		// For strings we put the value as a label with the same name as the metric.
 		// If the name is already an index, we do not need to set it again.
 		if _, ok := labels[metric.Name]; !ok {
 			labelnames = append(labelnames, metric.Name)
-			labelvalues = append(labelvalues, pduValueAsString(pdu, metricType))
+			labelvalues = append(labelvalues, pduValueAsString(pdu, metricType, metrics))
 		}
 	}
 
@@ -710,7 +728,7 @@ func splitOid(oid []int, count int) ([]int, []int) {
 }
 
 // This mirrors decodeValue in gosnmp's helper.go.
-func pduValueAsString(pdu *gosnmp.SnmpPDU, typ string) string {
+func pduValueAsString(pdu *gosnmp.SnmpPDU, typ string, metrics internalMetrics) string {
 	switch pdu.Value.(type) {
 	case int:
 		return strconv.Itoa(pdu.Value.(int))
@@ -748,7 +766,7 @@ func pduValueAsString(pdu *gosnmp.SnmpPDU, typ string) string {
 		return ""
 	default:
 		// This shouldn't happen.
-		snmpUnexpectedPduType.Inc()
+		metrics.snmpUnexpectedPduType.Inc()
 		return fmt.Sprintf("%s", pdu.Value)
 	}
 }
@@ -858,7 +876,7 @@ func indexOidsAsString(indexOids []int, typ string, fixedSize int, implied bool,
 	}
 }
 
-func indexesToLabels(indexOids []int, metric *config.Metric, oidToPdu map[string]gosnmp.SnmpPDU) map[string]string {
+func indexesToLabels(indexOids []int, metric *config.Metric, oidToPdu map[string]gosnmp.SnmpPDU, metrics internalMetrics) map[string]string {
 	labels := map[string]string{}
 	labelOids := map[string][]int{}
 
@@ -884,7 +902,7 @@ func indexesToLabels(indexOids []int, metric *config.Metric, oidToPdu map[string
 			oid = fmt.Sprintf("%s.%s", oid, listToOid(labelOids[label]))
 		}
 		if pdu, ok := oidToPdu[oid]; ok {
-			labels[lookup.Labelname] = pduValueAsString(&pdu, lookup.Type)
+			labels[lookup.Labelname] = pduValueAsString(&pdu, lookup.Type, metrics)
 			labelOids[lookup.Labelname] = []int{int(gosnmp.ToBigInt(pdu.Value).Int64())}
 		} else {
 			labels[lookup.Labelname] = ""
diff --git a/main.go b/main.go
index 31ea287..2cba003 100644
--- a/main.go
+++ b/main.go
@@ -124,7 +124,7 @@ func handler(w http.ResponseWriter, r *http.Request, logger log.Logger) {
 
 	start := time.Now()
 	registry := prometheus.NewRegistry()
-	c := collector.New(r.Context(), target, auth, module, logger)
+	c := collector.New(r.Context(), target, auth, module, logger, registry)
 	registry.MustRegister(c)
 	// Delegate http serving to Prometheus client library, which will call collector.Collect.
 	h := promhttp.HandlerFor(registry, promhttp.HandlerOpts{})


Test patch:
diff --git a/collector/collector_test.go b/collector/collector_test.go
index b2ed2af..7a5fae2 100644
--- a/collector/collector_test.go
+++ b/collector/collector_test.go
@@ -23,7 +23,7 @@ import (
 	kingpin "github.com/alecthomas/kingpin/v2"
 	"github.com/go-kit/log"
 	"github.com/gosnmp/gosnmp"
-	"github.com/prometheus/client_model/go"
+	io_prometheus_client "github.com/prometheus/client_model/go"
 
 	"github.com/prometheus/snmp_exporter/config"
 )
@@ -518,7 +518,7 @@ func TestPduToSample(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		metrics := pduToSamples(c.indexOids, c.pdu, c.metric, c.oidToPdu, log.NewNopLogger())
+		metrics := pduToSamples(c.indexOids, c.pdu, c.metric, c.oidToPdu, log.NewNopLogger(), internalMetrics{})
 		metric := &io_prometheus_client.Metric{}
 		expected := map[string]struct{}{}
 		for _, e := range c.expectedMetrics {
@@ -721,7 +721,7 @@ func TestPduValueAsString(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		got := pduValueAsString(c.pdu, c.typ)
+		got := pduValueAsString(c.pdu, c.typ, internalMetrics{})
 		if !reflect.DeepEqual(got, c.result) {
 			t.Errorf("pduValueAsString(%v, %q): got %q, want %q", c.pdu, c.typ, got, c.result)
 		}
@@ -1008,7 +1008,7 @@ func TestIndexesToLabels(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		got := indexesToLabels(c.oid, &c.metric, c.oidToPdu)
+		got := indexesToLabels(c.oid, &c.metric, c.oidToPdu, internalMetrics{})
 		if !reflect.DeepEqual(got, c.result) {
 			t.Errorf("indexesToLabels(%v, %v, %v): got %v, want %v", c.oid, c.metric, c.oidToPdu, got, c.result)
 		}
@@ -1059,7 +1059,7 @@ func TestFilterAllowedIndices(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		got := filterAllowedIndices(log.NewNopLogger(), c.filter, pdus, c.allowedList)
+		got := filterAllowedIndices(log.NewNopLogger(), c.filter, pdus, c.allowedList, internalMetrics{})
 		if !reflect.DeepEqual(got, c.result) {
 			t.Errorf("filterAllowedIndices(%v): got %v, want %v", c.filter, got, c.result)
 		}