Batch time series with different label values into same request. (#119)

* Batch timeseries with different label values into same request. * fixed one more review comment.
census-ecosystem · Apr 1, 2019 · dc2fe43 · dc2fe43
1 parent 0ca8377
commit dc2fe43
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 60 deletions.
diff --git a/metrics_proto.go b/metrics_proto.go
@@ -24,6 +24,8 @@ import (
 	"errors"
 	"fmt"
 	"path"
+	"sort"
+	"strings"
 
 	"github.com/golang/protobuf/ptypes/timestamp"
 	"go.opencensus.io/stats"
@@ -117,6 +119,20 @@ func (se *statsExporter) handleMetricsProtoUpload(payloads []*metricProtoPayload
 	return nil
 }
 
+// metricSignature creates a unique signature consisting of a
+// metric's type and its lexicographically sorted label values
+// See https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver/issues/120
+func metricSignature(metric *googlemetricpb.Metric) string {
+	labels := metric.GetLabels()
+	labelValues := make([]string, 0, len(labels))
+
+	for _, labelValue := range labels {
+		labelValues = append(labelValues, labelValue)
+	}
+	sort.Strings(labelValues)
+	return fmt.Sprintf("%s:%s", metric.GetType(), strings.Join(labelValues, ","))
+}
+
 func (se *statsExporter) combineTimeSeriesToCreateTimeSeriesRequest(ts []*monitoringpb.TimeSeries) (ctsreql []*monitoringpb.CreateTimeSeriesRequest) {
 	if len(ts) == 0 {
 		return nil
@@ -139,10 +155,10 @@ func (se *statsExporter) combineTimeSeriesToCreateTimeSeriesRequest(ts []*monito
 	seenMetrics := make(map[string]struct{})
 
 	for _, tti := range ts {
-		signature := tti.Metric.GetType()
-		if _, alreadySeen := seenMetrics[signature]; !alreadySeen {
+		key := metricSignature(tti.Metric)
+		if _, alreadySeen := seenMetrics[key]; !alreadySeen {
 			uniqueTimeSeries = append(uniqueTimeSeries, tti)
-			seenMetrics[signature] = struct{}{}
+			seenMetrics[key] = struct{}{}
 		} else {
 			nonUniqueTimeSeries = append(nonUniqueTimeSeries, tti)
 		}

diff --git a/metrics_proto_test.go b/metrics_proto_test.go
@@ -397,11 +397,17 @@ func TestCombineTimeSeriesAndDeduplication(t *testing.T) {
 				{
 					Metric: &googlemetricpb.Metric{
 						Type: "a/b/c",
+						Labels: map[string]string{
+							"k1": "v1",
+						},
 					},
 				},
 				{
 					Metric: &googlemetricpb.Metric{
 						Type: "a/b/c",
+						Labels: map[string]string{
+							"k1": "v2",
+						},
 					},
 				},
 				{
@@ -412,6 +418,9 @@ func TestCombineTimeSeriesAndDeduplication(t *testing.T) {
 				{
 					Metric: &googlemetricpb.Metric{
 						Type: "a/b/c",
+						Labels: map[string]string{
+							"k1": "v1",
+						},
 					},
 				},
 				{
@@ -427,26 +436,27 @@ func TestCombineTimeSeriesAndDeduplication(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type: "a/b/c",
+								Labels: map[string]string{
+									"k1": "v1",
+								},
 							},
 						},
 						{
 							Metric: &googlemetricpb.Metric{
-								Type: "A/b/c",
+								Type: "a/b/c",
+								Labels: map[string]string{
+									"k1": "v2",
+								},
 							},
 						},
 						{
 							Metric: &googlemetricpb.Metric{
-								Type: "X/Y/Z",
+								Type: "A/b/c",
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath(se.o.ProjectID),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &googlemetricpb.Metric{
-								Type: "a/b/c",
+								Type: "X/Y/Z",
 							},
 						},
 					},
@@ -457,6 +467,9 @@ func TestCombineTimeSeriesAndDeduplication(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type: "a/b/c",
+								Labels: map[string]string{
+									"k1": "v1",
+								},
 							},
 						},
 					},

diff --git a/stats_test.go b/stats_test.go
@@ -138,11 +138,6 @@ func TestExporter_makeReq(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/example.com/views/testview",
@@ -218,11 +213,6 @@ func TestExporter_makeReq(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "external.googleapis.com/example.com/views/testview",
@@ -293,11 +283,6 @@ func TestExporter_makeReq(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/example.com/views/testview",
@@ -364,11 +349,6 @@ func TestExporter_makeReq(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/lasttestview",
@@ -542,21 +522,21 @@ func TestExporter_makeReq_batching(t *testing.T) {
 			name:      "4 vds; 3 limit",
 			iter:      2,
 			limit:     3,
-			wantReqs:  4,
+			wantReqs:  3,
 			wantTotal: 4,
 		},
 		{
 			name:      "4 vds; 4 limit",
 			iter:      2,
 			limit:     4,
-			wantReqs:  4,
+			wantReqs:  2,
 			wantTotal: 4,
 		},
 		{
 			name:      "4 vds; 5 limit",
 			iter:      2,
 			limit:     5,
-			wantReqs:  4,
+			wantReqs:  2,
 			wantTotal: 4,
 		},
 	}
@@ -996,11 +976,6 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/testview",
@@ -1074,11 +1049,6 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/testview",
@@ -1154,11 +1124,6 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/testview",
@@ -1232,11 +1197,6 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/testview",
@@ -1303,11 +1263,6 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
 								},
 							},
 						},
-					},
-				},
-				{
-					Name: monitoring.MetricProjectPath("proj-id"),
-					TimeSeries: []*monitoringpb.TimeSeries{
 						{
 							Metric: &metricpb.Metric{
 								Type: "custom.googleapis.com/opencensus/testview",
@@ -1414,7 +1369,7 @@ func TestExporter_customContext(t *testing.T) {
 	if ctx.Err() != context.DeadlineExceeded {
 		t.Errorf("expected context to time out; got %v", ctx.Err())
 	}
-	if timedOut != 3 {
+	if timedOut != 2 {
 		t.Errorf("expected two functions to time out; got %d", timedOut)
 	}
 }