remove noop cache, add helper function for tests

Signed-off-by: glightfoot <glightfoot@rsglab.com>

main.go

@@ -253,7 +253,7 @@ func getCache(cacheSize int, cacheType string, registerer prometheus.Registerer)
 	var cache mapper.MetricMapperCache
 	var err error
 	if cacheSize == 0 {
-		cache = mapper.NewMetricMapperNoopCache()
+		return nil, nil
 	} else {
 		switch cacheType {
 		case "lru":

pkg/exporter/exporter_test.go

@@ -182,7 +182,6 @@ func TestNegativeCounter(t *testing.T) {
 	prev := getTelemetryCounterValue(errorCounter)
 
 	testMapper := mapper.MetricMapper{}
-	testMapper.UseCache(mapper.NewMetricMapperNoopCache())
 
 	ex := NewExporter(prometheus.DefaultRegisterer, &testMapper, log.NewNopLogger(), eventsActions, eventsUnmapped, errorEventStats, eventStats, conflictingEventStats, metricsCount)
 	ex.Listen(events)
@@ -318,7 +317,6 @@ mappings:
 `
 
 	testMapper := &mapper.MetricMapper{}
-	testMapper.UseCache(mapper.NewMetricMapperNoopCache())
 	err := testMapper.InitFromYAMLString(config)
 	if err != nil {
 		t.Fatalf("Config load error: %s %s", config, err)

pkg/mapper/mapper.go

@@ -229,12 +229,10 @@ func (m *MetricMapper) InitFromYAMLString(fileContents string) error {
 	m.Defaults = n.Defaults
 	m.Mappings = n.Mappings
 
-	// If no cache has been configured, use a noop cache
-	if m.cache == nil {
-		m.cache = NewMetricMapperNoopCache()
-	}
 	// Reset the cache since this function can be used to reload config
+	if m.cache != nil {
 		m.cache.Reset()
+	}
 
 	if n.doFSM {
 		var mappings []string
@@ -266,6 +264,8 @@ func (m *MetricMapper) InitFromFile(fileName string) error {
 }
 
 func (m *MetricMapper) UseCache(cache MetricMapperCache) {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
 	m.cache = cache
 }
 
@@ -273,16 +273,14 @@ func (m *MetricMapper) GetMapping(statsdMetric string, statsdMetricType MetricTy
 	m.mutex.RLock()
 	defer m.mutex.RUnlock()
 
-	// default cache to noop cache if used from an uninitialized mapper
+	// only use a cache if one is present
-	if m.cache == nil {
+	if m.cache != nil {
-		m.cache = NewMetricMapperNoopCache()
-	}
-
 		result, cached := m.cache.Get(formatKey(statsdMetric, statsdMetricType))
 		if cached {
 			r := result.(MetricMapperCacheResult)
 			return r.Mapping, r.Labels, r.Matched
 		}
+	}
 
 	// glob matching
 	if m.doFSM {
@@ -303,13 +301,17 @@ func (m *MetricMapper) GetMapping(statsdMetric string, statsdMetricType MetricTy
 				Labels:  labels,
 			}
 			// add match to cache
+			if m.cache != nil {
 				m.cache.Add(formatKey(statsdMetric, statsdMetricType), r)
+			}
 
 			return result, labels, true
 		} else if !m.doRegex {
 			// if there's no regex match type, return immediately
-			// Add miss cache
+			// Add miss to cache
+			if m.cache != nil {
 				m.cache.Add(formatKey(statsdMetric, statsdMetricType), MetricMapperCacheResult{})
+			}
 			return nil, nil, false
 		}
 	}
@@ -348,13 +350,17 @@ func (m *MetricMapper) GetMapping(statsdMetric string, statsdMetricType MetricTy
 			Labels:  labels,
 		}
 		// Add Match to cache
+		if m.cache != nil {
 			m.cache.Add(formatKey(statsdMetric, statsdMetricType), r)
+		}
 
 		return &mapping, labels, true
 	}
 
 	// Add Miss to cache
+	if m.cache != nil {
 		m.cache.Add(formatKey(statsdMetric, statsdMetricType), MetricMapperCacheResult{})
+	}
 	return nil, nil, false
 }
 

pkg/mapper/mapper_benchmark_test.go

@@ -585,15 +585,7 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob)
 	}
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -700,15 +692,7 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob)
 	}
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -845,15 +829,7 @@ mappings:` + duplicateRules(100, ruleTemplateMultipleMatchGlob)
 	}
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -927,15 +903,7 @@ mappings:` + duplicateRules(100, ruleTemplateMultipleMatchRegex)
 	}
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -968,16 +936,7 @@ mappings:` + duplicateRules(101, ruleTemplateSingleMatchGlob)
 	mappings := duplicateMetrics(100, "metric100")
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -1006,15 +965,7 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob)
 	mappings := duplicateMetrics(100, "metric100")
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 1000)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 50)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 50)
-		}
-		mapper.UseCache(cache)
 
 		b.Run(cacheType, func(b *testing.B) {
 			err := mapper.InitFromYAMLString(config)
@@ -1050,18 +1001,8 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob)
 	})
 
 	for _, cacheType := range []string{"lru", "random"} {
-		mapper := MetricMapper{}
+		mapper := newTestMapperWithCache(cacheType, 50)
-		var cache MetricMapperCache
-		switch cacheType {
-		case "lru":
-			cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 50)
-		case "random":
-			cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 50)
-		}
-		mapper.UseCache(cache)
-
 		b.Run(cacheType, func(b *testing.B) {
-			mapper := MetricMapper{}
 			err := mapper.InitFromYAMLString(config)
 			if err != nil {
 				b.Fatalf("Config load error: %s %s", config, err)

pkg/mapper/mapper_cache.go

@@ -69,20 +69,6 @@ type MetricMapperCache interface {
 	Reset()
 }
 
-type MetricMapperNoopCache struct{}
-
-func NewMetricMapperNoopCache() *MetricMapperNoopCache {
-	return &MetricMapperNoopCache{}
-}
-
-func (m *MetricMapperNoopCache) Get(metricKey string) (interface{}, bool) {
-	return nil, false
-}
-
-func (m *MetricMapperNoopCache) Add(metricKey string, result interface{}) {}
-
-func (m *MetricMapperNoopCache) Reset() {}
-
 func formatKey(metricString string, metricType MetricType) string {
 	return string(metricType) + "." + metricString
 }

pkg/mapper/mapper_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/prometheus/statsd_exporter/pkg/mappercache/lru"
+	"github.com/prometheus/statsd_exporter/pkg/mappercache/randomreplacement"
 )
 
 type mappings []struct {
@@ -36,6 +37,21 @@ type mappings []struct {
 	buckets      []float64
 }
 
+func newTestMapperWithCache(cacheType string, size int) *MetricMapper {
+	mapper := MetricMapper{}
+	var cache MetricMapperCache
+	switch cacheType {
+	case "lru":
+		cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, size)
+	case "random":
+		cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, size)
+	case "none":
+		return &mapper
+	}
+	mapper.UseCache(cache)
+	return &mapper
+}
+
 func TestMetricMapperYAML(t *testing.T) {
 	scenarios := []struct {
 		testName  string
@@ -1575,11 +1591,6 @@ mappings:
   labels:
     app: "$2"
 `
-	mapper := MetricMapper{}
-	err := mapper.InitFromYAMLString(config)
-	if err != nil {
-		t.Fatalf("config load error: %s ", err)
-	}
 
 	names := map[string]string{
 		"aa.bb.aa.myapp": "aa_bb_aa_total",
@@ -1588,24 +1599,14 @@ mappings:
 		"aa.bb.dd.myapp": "aa_bb_dd_total",
 	}
 
-	lruCache, err := lru.NewMetricMapperLRUCache(mapper.Registerer, len(names))
+	scenarios := []string{"none", "lru"}
-	if err != nil {
-		t.Fatalf(err.Error())
-	}
-
-	scenarios := []struct {
-		cache MetricMapperCache
-	}{
-		{
-			cache: NewMetricMapperNoopCache(),
-		},
-		{
-			cache: lruCache,
-		},
-	}
 
 	for i, scenario := range scenarios {
-		mapper.UseCache(scenario.cache)
+		mapper := newTestMapperWithCache(scenario, 1000)
+		err := mapper.InitFromYAMLString(config)
+		if err != nil {
+			t.Fatalf("config load error: %s ", err)
+		}
 
 		// run multiple times to ensure cache works as expected
 		for j := 0; j < 10; j++ {