From 35559538c7d1caa7b8bb623247a0b2cc80298f49 Mon Sep 17 00:00:00 2001
From: "Stephen McQuay (smcquay)" <smcquay@wish.com>
Date: Fri, 15 Dec 2017 11:00:37 -0800
Subject: [PATCH] Implements review commentary

Specifically @beorn7 pointed out that the previous implementation had
some shortcomings around large numbers. I've changed the code to match
the suggestion in review, as well as added a few test cases.
---
 prometheus/counter_test.go | 84 ++++++++++++++++++++++++++++++++++++++
 prometheus/value.go        |  6 +--
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go
index b5eded9..0b79728 100644
--- a/prometheus/counter_test.go
+++ b/prometheus/counter_test.go
@@ -126,3 +126,87 @@ func expectPanic(t *testing.T, op func(), errorMsg string) {
 
 	op()
 }
+
+func TestCounterAddInf(t *testing.T) {
+	counter := NewCounter(CounterOpts{
+		Name: "test",
+		Help: "test help",
+	}).(*counter)
+
+	counter.Inc()
+	if expected, got := 0.0, math.Float64frombits(counter.valBits); expected != got {
+		t.Errorf("Expected %f, got %f.", expected, got)
+	}
+	if expected, got := int64(1), counter.valInt; expected != got {
+		t.Errorf("Expected %f, got %f.", expected, got)
+	}
+
+	counter.Add(math.Inf(1))
+	if expected, got := math.Inf(1), math.Float64frombits(counter.valBits); expected != got {
+		t.Errorf("valBits expected %f, got %f.", expected, got)
+	}
+	if expected, got := int64(1), counter.valInt; expected != got {
+		t.Errorf("valInts expected %d, got %d.", expected, got)
+	}
+
+	counter.Inc()
+	if expected, got := math.Inf(1), math.Float64frombits(counter.valBits); expected != got {
+		t.Errorf("Expected %f, got %f.", expected, got)
+	}
+	if expected, got := int64(2), counter.valInt; expected != got {
+		t.Errorf("Expected %d, got %d.", expected, got)
+	}
+
+	m := &dto.Metric{}
+	counter.Write(m)
+
+	if expected, got := `counter:<value:inf > `, m.String(); expected != got {
+		t.Errorf("expected %q, got %q", expected, got)
+	}
+}
+
+func TestCounterAddLarge(t *testing.T) {
+	counter := NewCounter(CounterOpts{
+		Name: "test",
+		Help: "test help",
+	}).(*counter)
+
+	// large overflows the underlying type and should therefore be stored in valBits
+	large := float64(math.MaxInt64 + 1)
+	counter.Add(large)
+	if expected, got := large, math.Float64frombits(counter.valBits); expected != got {
+		t.Errorf("valBits expected %f, got %f.", expected, got)
+	}
+	if expected, got := int64(0), counter.valInt; expected != got {
+		t.Errorf("valInts expected %d, got %d.", expected, got)
+	}
+
+	m := &dto.Metric{}
+	counter.Write(m)
+
+	if expected, got := fmt.Sprintf("counter:<value:%0.15e > ", large), m.String(); expected != got {
+		t.Errorf("expected %q, got %q", expected, got)
+	}
+}
+
+func TestCounterAddSmall(t *testing.T) {
+	counter := NewCounter(CounterOpts{
+		Name: "test",
+		Help: "test help",
+	}).(*counter)
+	small := 0.000000000001
+	counter.Add(small)
+	if expected, got := small, math.Float64frombits(counter.valBits); expected != got {
+		t.Errorf("valBits expected %f, got %f.", expected, got)
+	}
+	if expected, got := int64(0), counter.valInt; expected != got {
+		t.Errorf("valInts expected %d, got %d.", expected, got)
+	}
+
+	m := &dto.Metric{}
+	counter.Write(m)
+
+	if expected, got := fmt.Sprintf("counter:<value:%0.0e > ", small), m.String(); expected != got {
+		t.Errorf("expected %q, got %q", expected, got)
+	}
+}
diff --git a/prometheus/value.go b/prometheus/value.go
index 23a1521..5626e80 100644
--- a/prometheus/value.go
+++ b/prometheus/value.go
@@ -45,7 +45,6 @@ type value struct {
 	// to go first in the struct to guarantee alignment for atomic
 	// operations.  http://golang.org/pkg/sync/atomic/#pkg-note-BUG
 	valBits uint64
-
 	// valInt is used to store values that are exact integers
 	valInt int64
 
@@ -99,8 +98,9 @@ func (v *value) Dec() {
 }
 
 func (v *value) Add(val float64) {
-	if math.Trunc(val) == val {
-		atomic.AddInt64(&v.valInt, int64(val))
+	ival := int64(val)
+	if float64(ival) == val {
+		v.add(ival)
 		return
 	}