From db4be855787c77512b2feaee6753e030ee1892d6 Mon Sep 17 00:00:00 2001
From: Toni Reina <treina@newrelic.com>
Date: Thu, 4 Jun 2020 18:25:02 +0200
Subject: [PATCH] Update Summary metrics to support null min/max values

As described in the spec:
https://github.com/newrelic/newrelic-telemetry-sdk-specs/blob/master/capabilities.md#summary

Summary metrics could contain null values in min or max fields. This
change adds support to instantiate Summary metrics using NaN values for
min/max fields.
---
 telemetry/metrics.go      | 29 +++++++++++++++++++++++++----
 telemetry/metrics_test.go | 35 +++++++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/telemetry/metrics.go b/telemetry/metrics.go
index 3762c01..3af1c5b 100644
--- 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
index 5b1ce65..141524f 100644
--- a/telemetry/metrics_test.go
+++ b/telemetry/metrics_test.go
@@ -22,6 +22,15 @@ func TestMetricPayload(t *testing.T) {
 		Timestamp:  now,
 		Value:      1.0,
 	})
+	h.RecordMetric(Summary{
+		Name:       "another-metric",
+		Attributes: map[string]interface{}{"zip": "zap"},
+		Timestamp:  now,
+		Count:      4.0,
+		Sum:        1.0,
+		Min:        math.NaN(),
+		Max:        3.0,
+	})
 	h.lastHarvest = now
 	end := h.lastHarvest.Add(5 * time.Second)
 	reqs := h.swapOutMetrics(end)
@@ -37,7 +46,8 @@ 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":"another-metric","type":"summary","value":{"sum":1,"count":4,"min":null,"max":3},"timestamp":1417136460000,"attributes":{"zip":"zap"}}
 		]
 	}]`
 	compactExpect := compactJSONString(expect)
@@ -151,11 +161,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 +179,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 {