From 8b306c8c7669b0e08ba227e72ae64f025a8d7d76 Mon Sep 17 00:00:00 2001 From: glightfoot Date: Fri, 19 Feb 2021 11:17:27 -0500 Subject: [PATCH] remove noop cache, add helper function for tests Signed-off-by: glightfoot --- main.go | 2 +- pkg/exporter/exporter_test.go | 2 - pkg/mapper/mapper.go | 44 +++++++++-------- pkg/mapper/mapper_benchmark_test.go | 73 +++-------------------------- pkg/mapper/mapper_cache.go | 14 ------ pkg/mapper/mapper_test.go | 43 ++++++++--------- 6 files changed, 55 insertions(+), 123 deletions(-) diff --git a/main.go b/main.go index b6b0c8d..66487a9 100644 --- a/main.go +++ b/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": diff --git a/pkg/exporter/exporter_test.go b/pkg/exporter/exporter_test.go index bc17648..3948f76 100644 --- a/pkg/exporter/exporter_test.go +++ b/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) diff --git a/pkg/mapper/mapper.go b/pkg/mapper/mapper.go index 302a61f..e610e1a 100644 --- a/pkg/mapper/mapper.go +++ b/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 - m.cache.Reset() + 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,15 +273,13 @@ 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 - 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 + // only use a cache if one is present + if m.cache != nil { + result, cached := m.cache.Get(formatKey(statsdMetric, statsdMetricType)) + if cached { + r := result.(MetricMapperCacheResult) + return r.Mapping, r.Labels, r.Matched + } } // glob matching @@ -303,13 +301,17 @@ func (m *MetricMapper) GetMapping(statsdMetric string, statsdMetricType MetricTy Labels: labels, } // add match to cache - m.cache.Add(formatKey(statsdMetric, statsdMetricType), r) + 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 - m.cache.Add(formatKey(statsdMetric, statsdMetricType), MetricMapperCacheResult{}) + // 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 - m.cache.Add(formatKey(statsdMetric, statsdMetricType), r) + if m.cache != nil { + m.cache.Add(formatKey(statsdMetric, statsdMetricType), r) + } return &mapping, labels, true } // Add Miss to cache - m.cache.Add(formatKey(statsdMetric, statsdMetricType), MetricMapperCacheResult{}) + if m.cache != nil { + m.cache.Add(formatKey(statsdMetric, statsdMetricType), MetricMapperCacheResult{}) + } return nil, nil, false } diff --git a/pkg/mapper/mapper_benchmark_test.go b/pkg/mapper/mapper_benchmark_test.go index 978a110..e8adeee 100644 --- a/pkg/mapper/mapper_benchmark_test.go +++ b/pkg/mapper/mapper_benchmark_test.go @@ -585,15 +585,7 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob) } for _, cacheType := range []string{"lru", "random"} { - mapper := MetricMapper{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 1000) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 1000) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 50) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 50) - } - mapper.UseCache(cache) + mapper := newTestMapperWithCache(cacheType, 1000) 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{} - var cache MetricMapperCache - switch cacheType { - case "lru": - cache, _ = lru.NewMetricMapperLRUCache(mapper.Registerer, 50) - case "random": - cache, _ = randomreplacement.NewMetricMapperRRCache(mapper.Registerer, 50) - } - mapper.UseCache(cache) - + mapper := newTestMapperWithCache(cacheType, 50) 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) diff --git a/pkg/mapper/mapper_cache.go b/pkg/mapper/mapper_cache.go index 4d98e75..2c37fa3 100644 --- a/pkg/mapper/mapper_cache.go +++ b/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 } diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go index 8024b22..3aa6d51 100644 --- a/pkg/mapper/mapper_test.go +++ b/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)) - if err != nil { - t.Fatalf(err.Error()) - } - - scenarios := []struct { - cache MetricMapperCache - }{ - { - cache: NewMetricMapperNoopCache(), - }, - { - cache: lruCache, - }, - } + scenarios := []string{"none", "lru"} 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++ {