diff --git a/exporter.go b/exporter.go index 3e8359c..220198d 100644 --- a/exporter.go +++ b/exporter.go @@ -57,6 +57,19 @@ func (u uncheckedCollector) Collect(c chan<- prometheus.Metric) { u.c.Collect(c) } +type metricType int + +const ( + CounterMetricType metricType = iota + GaugeMetricType + SummaryMetricType + HistogramMetricType +) + +type metricChecker interface { + metricConflicts(string, metricType) bool +} + func getLabelNames(labels prometheus.Labels) []string { names := make([]string, 0, len(labels)) for labelName := range labels { @@ -96,12 +109,15 @@ func NewCounterContainer() *CounterContainer { } } -func (c *CounterContainer) Get(metricName string, labels prometheus.Labels, help string) (prometheus.Counter, error) { +func (c *CounterContainer) Get(metricName string, labels prometheus.Labels, mc metricChecker, help string) (prometheus.Counter, error) { labelNames := getLabelNames(labels) mapKey := getContainerMapKey(metricName, labelNames) counterVec, ok := c.Elements[mapKey] if !ok { + if mc.metricConflicts(metricName, CounterMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } metricsCount.WithLabelValues("counter").Inc() counterVec = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: metricName, @@ -134,12 +150,15 @@ func NewGaugeContainer() *GaugeContainer { } } -func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels, help string) (prometheus.Gauge, error) { +func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels, mc metricChecker, help string) (prometheus.Gauge, error) { labelNames := getLabelNames(labels) mapKey := getContainerMapKey(metricName, labelNames) gaugeVec, ok := c.Elements[mapKey] if !ok { + if mc.metricConflicts(metricName, GaugeMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } metricsCount.WithLabelValues("gauge").Inc() gaugeVec = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Name: metricName, @@ -174,12 +193,21 @@ func NewSummaryContainer(mapper *mapper.MetricMapper) *SummaryContainer { } } -func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels, help string, mapping *mapper.MetricMapping) (prometheus.Observer, error) { +func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels, mc metricChecker, help string, mapping *mapper.MetricMapping) (prometheus.Observer, error) { labelNames := getLabelNames(labels) mapKey := getContainerMapKey(metricName, labelNames) summaryVec, ok := c.Elements[mapKey] if !ok { + if mc.metricConflicts(metricName, SummaryMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } + if mc.metricConflicts(metricName+"_sum", SummaryMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } + if mc.metricConflicts(metricName+"_count", SummaryMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } metricsCount.WithLabelValues("summary").Inc() quantiles := c.mapper.Defaults.Quantiles if mapping != nil && mapping.Quantiles != nil && len(mapping.Quantiles) > 0 { @@ -228,12 +256,21 @@ func NewHistogramContainer(mapper *mapper.MetricMapper) *HistogramContainer { } } -func (c *HistogramContainer) Get(metricName string, labels prometheus.Labels, help string, mapping *mapper.MetricMapping) (prometheus.Observer, error) { +func (c *HistogramContainer) Get(metricName string, labels prometheus.Labels, mc metricChecker, help string, mapping *mapper.MetricMapping) (prometheus.Observer, error) { labelNames := getLabelNames(labels) mapKey := getContainerMapKey(metricName, labelNames) histogramVec, ok := c.Elements[mapKey] if !ok { + if mc.metricConflicts(metricName+"_sum", HistogramMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } + if mc.metricConflicts(metricName+"_count", HistogramMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } + if mc.metricConflicts(metricName+"_bucket", HistogramMetricType) { + return nil, fmt.Errorf("metric with name %s is already registered", metricName) + } metricsCount.WithLabelValues("histogram").Inc() buckets := c.mapper.Defaults.Buckets if mapping != nil && mapping.Buckets != nil && len(mapping.Buckets) > 0 { @@ -309,6 +346,7 @@ type LabelValues struct { lastRegisteredAt time.Time labels prometheus.Labels ttl time.Duration + metricType metricType } type Exporter struct { @@ -424,11 +462,12 @@ func (b *Exporter) handleEvent(event Event) { counter, err := b.Counters.Get( metricName, prometheusLabels, + b, help, ) if err == nil { counter.Add(event.Value()) - b.saveLabelValues(metricName, prometheusLabels, mapping.Ttl) + b.saveLabelValues(metricName, CounterMetricType, prometheusLabels, mapping.Ttl) eventStats.WithLabelValues("counter").Inc() } else { log.Debugf(regErrF, metricName, err) @@ -439,6 +478,7 @@ func (b *Exporter) handleEvent(event Event) { gauge, err := b.Gauges.Get( metricName, prometheusLabels, + b, help, ) @@ -448,7 +488,7 @@ func (b *Exporter) handleEvent(event Event) { } else { gauge.Set(event.Value()) } - b.saveLabelValues(metricName, prometheusLabels, mapping.Ttl) + b.saveLabelValues(metricName, GaugeMetricType, prometheusLabels, mapping.Ttl) eventStats.WithLabelValues("gauge").Inc() } else { log.Debugf(regErrF, metricName, err) @@ -469,12 +509,13 @@ func (b *Exporter) handleEvent(event Event) { histogram, err := b.Histograms.Get( metricName, prometheusLabels, + b, help, mapping, ) if err == nil { histogram.Observe(event.Value() / 1000) // prometheus presumes seconds, statsd millisecond - b.saveLabelValues(metricName, prometheusLabels, mapping.Ttl) + b.saveLabelValues(metricName, HistogramMetricType, prometheusLabels, mapping.Ttl) eventStats.WithLabelValues("timer").Inc() } else { log.Debugf(regErrF, metricName, err) @@ -485,12 +526,13 @@ func (b *Exporter) handleEvent(event Event) { summary, err := b.Summaries.Get( metricName, prometheusLabels, + b, help, mapping, ) if err == nil { summary.Observe(event.Value() / 1000) // prometheus presumes seconds, statsd millisecond - b.saveLabelValues(metricName, prometheusLabels, mapping.Ttl) + b.saveLabelValues(metricName, SummaryMetricType, prometheusLabels, mapping.Ttl) eventStats.WithLabelValues("timer").Inc() } else { log.Debugf(regErrF, metricName, err) @@ -528,7 +570,7 @@ func (b *Exporter) removeStaleMetrics() { } // saveLabelValues stores label values set to labelValues and update lastRegisteredAt time and ttl value -func (b *Exporter) saveLabelValues(metricName string, labels prometheus.Labels, ttl time.Duration) { +func (b *Exporter) saveLabelValues(metricName string, metricType metricType, labels prometheus.Labels, ttl time.Duration) { metric, hasMetric := b.labelValues[metricName] if !hasMetric { metric = make(map[uint64]*LabelValues) @@ -538,8 +580,9 @@ func (b *Exporter) saveLabelValues(metricName string, labels prometheus.Labels, metricLabelValues, ok := metric[hash] if !ok { metricLabelValues = &LabelValues{ - labels: labels, - ttl: ttl, + labels: labels, + ttl: ttl, + metricType: metricType, } b.labelValues[metricName][hash] = metricLabelValues } @@ -549,6 +592,29 @@ func (b *Exporter) saveLabelValues(metricName string, labels prometheus.Labels, metricLabelValues.ttl = ttl } +func (b *Exporter) metricConflicts(metricName string, metricType metricType) bool { + metric, hasMetric := b.labelValues[metricName] + if !hasMetric { + // No metric with this name exists + return false + } + + // The metric does exist. All metrics in the hash should be of the same + // type, so we pick check the first one we find in the hash to check the + // type. + for _, lvs := range metric { + if lvs.metricType == metricType { + // We've found a copy of this metric with this type, but different + // labels, so it's safe to create a new one. + return false + } + } + + // The metric exists, but it's of a different type than we're trying to + // create. + return true +} + func NewExporter(mapper *mapper.MetricMapper) *Exporter { return &Exporter{ Counters: NewCounterContainer(), diff --git a/exporter_test.go b/exporter_test.go index 0878e55..9abfa27 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -157,6 +157,213 @@ mappings: } } +// TestConflictingMetrics validates that the exporter will not register metrics +// of different types that have overlapping names. +func TestConflictingMetrics(t *testing.T) { + scenarios := []struct { + name string + expected []float64 + in Events + }{ + { + name: "counter vs gauge", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "cvg_test", + value: 1, + }, + &GaugeEvent{ + metricName: "cvg_test", + value: 2, + }, + }, + }, + { + name: "counter vs gauge with different labels", + expected: []float64{1, 2}, + in: Events{ + &CounterEvent{ + metricName: "cvgl_test", + value: 1, + labels: map[string]string{"tag": "1"}, + }, + &CounterEvent{ + metricName: "cvgl_test", + value: 2, + labels: map[string]string{"tag": "2"}, + }, + &GaugeEvent{ + metricName: "cvgl_test", + value: 3, + labels: map[string]string{"tag": "1"}, + }, + }, + }, + { + name: "counter vs gauge with same labels", + expected: []float64{3}, + in: Events{ + &CounterEvent{ + metricName: "cvgsl_test", + value: 1, + labels: map[string]string{"tag": "1"}, + }, + &CounterEvent{ + metricName: "cvgsl_test", + value: 2, + labels: map[string]string{"tag": "1"}, + }, + &GaugeEvent{ + metricName: "cvgsl_test", + value: 3, + labels: map[string]string{"tag": "1"}, + }, + }, + }, + { + name: "gauge vs counter", + expected: []float64{2}, + in: Events{ + &GaugeEvent{ + metricName: "gvc_test", + value: 2, + }, + &CounterEvent{ + metricName: "gvc_test", + value: 1, + }, + }, + }, + { + name: "counter vs histogram sum", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "histogram_test1_sum", + value: 1, + }, + &TimerEvent{ + metricName: "histogram.test1", + value: 2, + }, + }, + }, + { + name: "counter vs histogram count", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "histogram_test2_count", + value: 1, + }, + &TimerEvent{ + metricName: "histogram.test2", + value: 2, + }, + }, + }, + { + name: "counter vs histogram bucket", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "histogram_test3_bucket", + value: 1, + }, + &TimerEvent{ + metricName: "histogram.test3", + value: 2, + }, + }, + }, + { + name: "counter vs summary quantile", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "cvsq_test", + value: 1, + }, + &TimerEvent{ + metricName: "cvsq_test", + value: 2, + }, + }, + }, + { + name: "counter vs summary count", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "cvsc_count", + value: 1, + }, + &TimerEvent{ + metricName: "cvsc", + value: 2, + }, + }, + }, + { + name: "counter vs summary sum", + expected: []float64{1}, + in: Events{ + &CounterEvent{ + metricName: "cvss_sum", + value: 1, + }, + &TimerEvent{ + metricName: "cvss", + value: 2, + }, + }, + }, + } + + config := ` +mappings: +- match: histogram.* + timer_type: histogram + name: "histogram_${1}" +` + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + testMapper := &mapper.MetricMapper{} + err := testMapper.InitFromYAMLString(config) + if err != nil { + t.Fatalf("Config load error: %s %s", config, err) + } + + events := make(chan Events) + go func() { + events <- s.in + close(events) + }() + ex := NewExporter(testMapper) + ex.Listen(events) + + metrics, err := prometheus.DefaultGatherer.Gather() + if err != nil { + t.Fatalf("Cannot gather from DefaultGatherer: %v", err) + } + + for i, e := range s.expected { + mn := s.in[i].MetricName() + m := getFloat64(metrics, mn, s.in[i].Labels()) + + if m == nil { + t.Fatalf("Could not find time series with metric name '%v'", mn) + } + + if *m != e { + t.Fatalf("Expected to get %v, but got %v instead", e, *m) + } + } + }) + } +} + // TestEmptyStringMetric validates when a metric name ends up // being the empty string after applying the match replacements // tha we don't panic the Exporter Listener.