From 96ad48e4b0609d1c1d78ce953e1c3dbbd4337893 Mon Sep 17 00:00:00 2001
From: Nathan Dias <nathan.dias@orijtech.com>
Date: Tue, 19 Nov 2019 01:37:28 -0600
Subject: [PATCH] internal/telemetry/export/ocagent: correctly marshal points
 to JSON

This change adds a custom MarshalJSON func to Point so that values
are formatted the same way jsonpb formats them. This allows the
OpenCensus service to determine the concrete type of the point's
value when unmarshaling.

What works:
* Points with Int64, Double, and Distribution values will marshal correctly.

What does not work:
* Points with Summary values will not marshal.

Updates golang/go#33819

Change-Id: Ia76ebff4e0e2b6ff2ddf72b8d6187f49069d4cad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207838
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
---
 .../telemetry/export/ocagent/wire/metrics.go  | 47 ++++++++++++
 .../export/ocagent/wire/metrics_test.go       | 76 +++++++++++++++++++
 2 files changed, 123 insertions(+)
 create mode 100644 internal/telemetry/export/ocagent/wire/metrics_test.go

diff --git a/internal/telemetry/export/ocagent/wire/metrics.go b/internal/telemetry/export/ocagent/wire/metrics.go
index ea82becfd2..d9ff519f10 100644
--- a/internal/telemetry/export/ocagent/wire/metrics.go
+++ b/internal/telemetry/export/ocagent/wire/metrics.go
@@ -4,6 +4,11 @@
 
 package wire
 
+import (
+	"encoding/json"
+	"fmt"
+)
+
 type ExportMetricsServiceRequest struct {
 	Node     *Node     `json:"node,omitempty"`
 	Metrics  []*Metric `json:"metrics,omitempty"`
@@ -62,6 +67,48 @@ type PointInt64Value struct {
 	Int64Value int64 `json:"int64Value,omitempty"`
 }
 
+// MarshalJSON creates JSON formatted the same way as jsonpb so that the
+// OpenCensus service can correctly determine the underlying value type.
+// This custom MarshalJSON exists because,
+// by default *Point is JSON marshalled as:
+//     {"value": {"int64Value": 1}}
+// but it should be marshalled as:
+//     {"int64Value": 1}
+func (p *Point) MarshalJSON() ([]byte, error) {
+	if p == nil {
+		return []byte("null"), nil
+	}
+
+	switch d := p.Value.(type) {
+	case PointInt64Value:
+		return json.Marshal(&struct {
+			Timestamp *Timestamp `json:"timestamp,omitempty"`
+			Value     int64      `json:"int64Value,omitempty"`
+		}{
+			Timestamp: p.Timestamp,
+			Value:     d.Int64Value,
+		})
+	case PointDoubleValue:
+		return json.Marshal(&struct {
+			Timestamp *Timestamp `json:"timestamp,omitempty"`
+			Value     float64    `json:"doubleValue,omitempty"`
+		}{
+			Timestamp: p.Timestamp,
+			Value:     d.DoubleValue,
+		})
+	case PointDistributionValue:
+		return json.Marshal(&struct {
+			Timestamp *Timestamp         `json:"timestamp,omitempty"`
+			Value     *DistributionValue `json:"distributionValue,omitempty"`
+		}{
+			Timestamp: p.Timestamp,
+			Value:     d.DistributionValue,
+		})
+	default:
+		return nil, fmt.Errorf("unknown point type %T", p.Value)
+	}
+}
+
 type PointDoubleValue struct {
 	DoubleValue float64 `json:"doubleValue,omitempty"`
 }
diff --git a/internal/telemetry/export/ocagent/wire/metrics_test.go b/internal/telemetry/export/ocagent/wire/metrics_test.go
new file mode 100644
index 0000000000..b510ee29cc
--- /dev/null
+++ b/internal/telemetry/export/ocagent/wire/metrics_test.go
@@ -0,0 +1,76 @@
+package wire
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestMarshalJSON(t *testing.T) {
+	tests := []struct {
+		name string
+		pt   *Point
+		want string
+	}{
+		{
+			"PointInt64",
+			&Point{
+				Value: PointInt64Value{
+					Int64Value: 5,
+				},
+			},
+			`{"int64Value":5}`,
+		},
+		{
+			"PointDouble",
+			&Point{
+				Value: PointDoubleValue{
+					DoubleValue: 3.14,
+				},
+			},
+			`{"doubleValue":3.14}`,
+		},
+		{
+			"PointDistribution",
+			&Point{
+				Value: PointDistributionValue{
+					DistributionValue: &DistributionValue{
+						Count: 3,
+						Sum:   10,
+						Buckets: []*Bucket{
+							{
+								Count: 1,
+							},
+							{
+								Count: 2,
+							},
+						},
+						BucketOptions: BucketOptionsExplicit{
+							Bounds: []float64{
+								0, 5,
+							},
+						},
+					},
+				},
+			},
+			`{"distributionValue":{"count":3,"sum":10,"bucket_options":{"bounds":[0,5]},"buckets":[{"count":1},{"count":2}]}}`,
+		},
+		{
+			"nil point",
+			nil,
+			`null`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			buf, err := tt.pt.MarshalJSON()
+			if err != nil {
+				t.Fatalf("Got:\n%v\nWant:\n%v", err, nil)
+			}
+			got := string(buf)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Fatalf("Got:\n%s\nWant:\n%s", got, tt.want)
+			}
+		})
+	}
+}