Skip to content

Commit

Permalink
prometheus: prevent panic when incrmenting counter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lgfa29 committed May 9, 2023
1 parent b6d5c86 commit d9ca9af
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
7 changes: 7 additions & 0 deletions prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions prometheus/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d9ca9af

Please sign in to comment.