fixed review

collector/collector.go

@@ -44,7 +44,7 @@ var (
 	wrapCounters           = kingpin.Flag("snmp.wrap-large-counters", "Wrap 64-bit counters to avoid floating point rounding.").Default("true").Bool()
 	srcAddress             = kingpin.Flag("snmp.source-address", "Source address to send snmp from in the format 'address:port' to use when connecting targets. If the port parameter is empty or '0', as in '127.0.0.1:' or '[::1]:0', a source port number is automatically (random) chosen.").Default("").String()
 
-	SnmpDuration = promauto.NewHistogramVec(
+	snmpDuration = promauto.NewHistogramVec(
 		prometheus.HistogramOpts{
 			Name: "snmp_collection_duration_seconds",
 			Help: "Duration of collections by the SNMP exporter",
@@ -89,6 +89,10 @@ func listToOid(l []int) string {
 	return strings.Join(result, ".")
 }
 
+func InitModuleMetrics(auth, module string) {
+	snmpDuration.WithLabelValues(auth, module)
+}
+
 type ScrapeResults struct {
 	pdus    []gosnmp.SnmpPDU
 	packets uint64
@@ -514,7 +518,7 @@ func (c Collector) Collect(ch chan<- prometheus.Metric) {
 				c.collect(ch, m)
 				duration := time.Since(start).Seconds()
 				level.Debug(logger).Log("msg", "Finished scrape", "duration_seconds", duration)
-				SnmpDuration.WithLabelValues(c.authName, m.name).Observe(duration)
+				snmpDuration.WithLabelValues(c.authName, m.name).Observe(duration)
 			}
 		}()
 	}

main.go

@@ -92,6 +92,19 @@ func handler(w http.ResponseWriter, r *http.Request, logger log.Logger) {
 	if len(queryModule) == 0 {
 		queryModule = append(queryModule, "if_mib")
 	}
+	uniqueM := make(map[string]bool)
+	var modules []string
+	for _, qm := range queryModule {
+		for _, m := range strings.Split(qm, ",") {
+			if m == "" {
+				continue
+			}
+			if _, ok := uniqueM[m]; !ok {
+				uniqueM[m] = true
+				modules = append(modules, m)
+			}
+		}
+	}
 	sc.RLock()
 	auth, authOk := sc.C.Auths[authName]
 	if !authOk {
@@ -100,27 +113,21 @@ func handler(w http.ResponseWriter, r *http.Request, logger log.Logger) {
 		snmpRequestErrors.Inc()
 		return
 	}
-	uniqueM := make(map[string]bool)
-	var modules []*collector.NamedModule
-	for _, qm := range queryModule {
-		for _, m := range strings.Split(qm, ",") {
-			module, moduleOk := sc.C.Modules[m]
-			if !moduleOk {
-				sc.RUnlock()
-				http.Error(w, fmt.Sprintf("Unknown module '%s'", m), http.StatusBadRequest)
-				snmpRequestErrors.Inc()
-				return
-			}
-			if _, ok := uniqueM[m]; !ok {
-				uniqueM[m] = true
-				modules = append(modules, collector.NewNamedModule(m, module))
-			}
+	var nmodules []*collector.NamedModule
+	for _, m := range modules {
+		module, moduleOk := sc.C.Modules[m]
+		if !moduleOk {
+			sc.RUnlock()
+			http.Error(w, fmt.Sprintf("Unknown module '%s'", m), http.StatusBadRequest)
+			snmpRequestErrors.Inc()
+			return
 		}
+		nmodules = append(nmodules, collector.NewNamedModule(m, module))
 	}
 	sc.RUnlock()
 	logger = log.With(logger, "auth", authName, "target", target)
 	registry := prometheus.NewRegistry()
-	c := collector.New(r.Context(), target, authName, auth, modules, logger, registry, *concurrency)
+	c := collector.New(r.Context(), target, authName, auth, nmodules, logger, registry, *concurrency)
 	registry.MustRegister(c)
 	// Delegate http serving to Prometheus client library, which will call collector.Collect.
 	h := promhttp.HandlerFor(registry, promhttp.HandlerOpts{})
@@ -155,7 +162,7 @@ func (sc *SafeConfig) ReloadConfig(configFile string) (err error) {
 	// Initialize metrics.
 	for auth := range sc.C.Auths {
 		for module := range sc.C.Modules {
-			collector.SnmpDuration.WithLabelValues(auth, module)
+			collector.InitModuleMetrics(auth, module)
 		}
 	}
 	sc.Unlock()