From d9ca9af9f1f9c77de45535e74def9b8e8a62f5c4 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 26 Jan 2023 14:56:11 -0500 Subject: [PATCH] prometheus: prevent panic when incrmenting counter The Prometheus Counter.Add() method panics if the value being added is negative. Even if care is taken by consumers to never pass a negative value the panic could still happen due to float conversion or overflow. This change prevents go-metrics from causing consumers to crash. --- prometheus/prometheus.go | 7 +++++++ prometheus/prometheus_test.go | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index 5b2eeb4..88ae389 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -364,6 +364,13 @@ func (p *PrometheusSink) IncrCounterWithLabels(parts []string, val float32, labe key, hash := flattenKey(parts, labels) pc, ok := p.counters.Load(hash) + // Prometheus Counter.Add() panics if val < 0. We don't want this to + // cause applications to crash, so log an error instead. + if val < 0 { + log.Printf("[ERR] Attempting to increment Prometheus counter %v with value negative value %v", key, val) + return + } + // Does the counter exist? if ok { localCounter := *pc.(*counter) diff --git a/prometheus/prometheus_test.go b/prometheus/prometheus_test.go index f857367..f8a774d 100644 --- a/prometheus/prometheus_test.go +++ b/prometheus/prometheus_test.go @@ -168,6 +168,9 @@ func TestDefinitions(t *testing.T) { sink.AddSample(summaryDef.Name, 42) sink.IncrCounter(counterDef.Name, 1) + // Prometheus panic should not be propagated + sink.IncrCounter(counterDef.Name, -1) + // Test that the expiry behavior works as expected. First pick a time which // is after all the actual updates above. timeAfterUpdates := time.Now() @@ -359,6 +362,11 @@ func TestDefinitionsWithLabels(t *testing.T) { } return true }) + + // Prometheus panic should not be propagated + sink.IncrCounterWithLabels(counterDef.Name, -1, []metrics.Label{ + {Name: "version", Value: "some info"}, + }) } func TestMetricSinkInterface(t *testing.T) {