Merge pull request #14 from areina/summary-metric-nullable-min-max

Update Summary metrics to support null min/max values
newrelic · Jun 5, 2020 · bfa99a8 · bfa99a8
2 parents 9d3efb4 + ec7ecd6
commit bfa99a8
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 13 deletions.
diff --git a/telemetry/metrics.go b/telemetry/metrics.go
@@ -6,6 +6,7 @@ package telemetry
 import (
 	"bytes"
 	"encoding/json"
+	"math"
 	"time"
 
 	"github.com/newrelic/newrelic-telemetry-sdk-go/internal"
@@ -120,8 +121,6 @@ func (m Summary) validate() map[string]interface{} {
 	for _, v := range []float64{
 		m.Count,
 		m.Sum,
-		m.Min,
-		m.Max,
 	} {
 		if err := isFloatValid(v); err != nil {
 			return map[string]interface{}{
@@ -131,6 +130,20 @@ func (m Summary) validate() map[string]interface{} {
 			}
 		}
 	}
+
+	for _, v := range []float64{
+		m.Min,
+		m.Max,
+	} {
+		if math.IsInf(v, 0) {
+			return map[string]interface{}{
+				"message": "invalid summary field",
+				"name":    m.Name,
+				"err":     errFloatInfinity.Error(),
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -146,8 +159,16 @@ func (m Summary) writeJSON(buf *bytes.Buffer) {
 	vw := internal.JSONFieldsWriter{Buf: buf}
 	vw.FloatField("sum", m.Sum)
 	vw.FloatField("count", m.Count)
-	vw.FloatField("min", m.Min)
-	vw.FloatField("max", m.Max)
+	if math.IsNaN(m.Min) {
+		w.RawField("min", json.RawMessage(`null`))
+	} else {
+		vw.FloatField("min", m.Min)
+	}
+	if math.IsNaN(m.Max) {
+		vw.RawField("max", json.RawMessage(`null`))
+	} else {
+		vw.FloatField("max", m.Max)
+	}
 	buf.WriteByte('}')
 
 	writeTimestampInterval(&w, m.Timestamp, m.Interval)

diff --git a/telemetry/metrics_test.go b/telemetry/metrics_test.go
@@ -15,13 +15,30 @@ func TestMetricPayload(t *testing.T) {
 	// attributes correctly marshals into JSON.
 	now := time.Date(2014, time.November, 28, 1, 1, 0, 0, time.UTC)
 	h, _ := NewHarvester(ConfigCommonAttributes(map[string]interface{}{"zop": "zup"}), configTesting)
-	// Use a single metric to avoid sorting.
 	h.RecordMetric(Gauge{
 		Name:       "metric",
 		Attributes: map[string]interface{}{"zip": "zap"},
 		Timestamp:  now,
 		Value:      1.0,
 	})
+	h.RecordMetric(Summary{
+		Name:       "summary-metric-nan-min",
+		Attributes: map[string]interface{}{"zip": "zap"},
+		Timestamp:  now,
+		Count:      4.0,
+		Sum:        1.0,
+		Min:        math.NaN(),
+		Max:        3.0,
+	})
+	h.RecordMetric(Summary{
+		Name:       "summary-metric-nan-max",
+		Attributes: map[string]interface{}{"zip": "zap"},
+		Timestamp:  now,
+		Count:      4.0,
+		Sum:        1.0,
+		Min:        10,
+		Max:        math.NaN(),
+	})
 	h.lastHarvest = now
 	end := h.lastHarvest.Add(5 * time.Second)
 	reqs := h.swapOutMetrics(end)
@@ -37,7 +54,9 @@ func TestMetricPayload(t *testing.T) {
 			"attributes":{"zop":"zup"}
 		},
 		"metrics":[
-			{"name":"metric","type":"gauge","value":1,"timestamp":1417136460000,"attributes":{"zip":"zap"}}
+			{"name":"metric","type":"gauge","value":1,"timestamp":1417136460000,"attributes":{"zip":"zap"}},
+			{"name":"summary-metric-nan-min","type":"summary","value":{"sum":1,"count":4,"min":null,"max":3},"timestamp":1417136460000,"attributes":{"zip":"zap"}},
+			{"name":"summary-metric-nan-max","type":"summary","value":{"sum":1,"count":4,"min":10,"max":null},"timestamp":1417136460000,"attributes":{"zip":"zap"}}
 		]
 	}]`
 	compactExpect := compactJSONString(expect)
@@ -151,11 +170,16 @@ func TestValidateGauge(t *testing.T) {
 }
 
 func TestValidateSummary(t *testing.T) {
-	want := map[string]interface{}{
+	expectNaNErr := map[string]interface{}{
 		"message": "invalid summary field",
 		"name":    "my-summary",
 		"err":     errFloatNaN.Error(),
 	}
+	expectInfErr := map[string]interface{}{
+		"message": "invalid summary field",
+		"name":    "my-summary",
+		"err":     errFloatInfinity.Error(),
+	}
 	testcases := []struct {
 		m      Summary
 		fields map[string]interface{}
@@ -164,21 +188,25 @@ func TestValidateSummary(t *testing.T) {
 			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: 3.0, Max: 4.0},
 			fields: nil,
 		},
+		{
+			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: math.NaN(), Max: math.NaN()},
+			fields: nil,
+		},
 		{
 			m:      Summary{Name: "my-summary", Count: math.NaN(), Sum: 2.0, Min: 3.0, Max: 4.0},
-			fields: want,
+			fields: expectNaNErr,
 		},
 		{
 			m:      Summary{Name: "my-summary", Count: 1.0, Sum: math.NaN(), Min: 3.0, Max: 4.0},
-			fields: want,
+			fields: expectNaNErr,
 		},
 		{
-			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: math.NaN(), Max: 4.0},
-			fields: want,
+			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: math.Inf(-3), Max: 4.0},
+			fields: expectInfErr,
 		},
 		{
-			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: 3.0, Max: math.NaN()},
-			fields: want,
+			m:      Summary{Name: "my-summary", Count: 1.0, Sum: 2.0, Min: 3.0, Max: math.Inf(3)},
+			fields: expectInfErr,
 		},
 	}
 	for idx, tc := range testcases {