Small optimization to avoid unnecesarry allocations. (#228)

* Small optimization to avoid unnecesarry allocations. Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com> * Move timeout to all create metric descriptor. Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
census-ecosystem · Oct 5, 2019 · 8033da9 · 8033da9
1 parent 9cc5395
commit 8033da9
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 32 deletions.
diff --git a/metrics.go b/metrics.go
@@ -168,17 +168,21 @@ func (se *statsExporter) metricToMpbTs(ctx context.Context, metric *metricdata.M
 }
 
 func metricLabelsToTsLabels(defaults map[string]labelValue, labelKeys []metricdata.LabelKey, labelValues []metricdata.LabelValue) (map[string]string, error) {
+	// Perform this sanity check now.
+	if len(labelKeys) != len(labelValues) {
+		return nil, fmt.Errorf("length mismatch: len(labelKeys)=%d len(labelValues)=%d", len(labelKeys), len(labelValues))
+	}
+
+	if len(defaults)+len(labelKeys) == 0 {
+		return nil, nil
+	}
+
 	labels := make(map[string]string)
 	// Fill in the defaults firstly, irrespective of if the labelKeys and labelValues are mismatched.
 	for key, label := range defaults {
 		labels[sanitize(key)] = label.val
 	}
 
-	// Perform this sanity check now.
-	if len(labelKeys) != len(labelValues) {
-		return labels, fmt.Errorf("length mismatch: len(labelKeys)=%d len(labelValues)=%d", len(labelKeys), len(labelValues))
-	}
-
 	for i, labelKey := range labelKeys {
 		labelValue := labelValues[i]
 		labels[sanitize(labelKey.Key)] = labelValue.Value

diff --git a/metrics_proto.go b/metrics_proto.go
@@ -67,14 +67,14 @@ func (se *statsExporter) PushMetricsProto(ctx context.Context, node *commonpb.No
 		if metric.GetMetricDescriptor().GetType() == metricspb.MetricDescriptor_SUMMARY {
 			summaryMtcs := se.convertSummaryMetrics(metric)
 			for _, summaryMtc := range summaryMtcs {
-				if err := se.createMetricDescriptorWithTimeout(ctx, summaryMtc); err != nil {
+				if err := se.createMetricDescriptorFromMetricProto(ctx, summaryMtc); err != nil {
 					mb.recordDroppedTimeseries(len(summaryMtc.GetTimeseries()), err)
 					continue
 				}
 				se.protoMetricToTimeSeries(ctx, mappedRsc, summaryMtc, mb)
 			}
 		} else {
-			if err := se.createMetricDescriptorWithTimeout(ctx, metric); err != nil {
+			if err := se.createMetricDescriptorFromMetricProto(ctx, metric); err != nil {
 				mb.recordDroppedTimeseries(len(metric.GetTimeseries()), err)
 				continue
 			}
@@ -236,8 +236,7 @@ func (se *statsExporter) protoMetricToTimeSeries(ctx context.Context, mappedRsc
 		mb.recordDroppedTimeseries(len(metric.GetTimeseries()), errNilMetricOrMetricDescriptor)
 	}
 
-	metricName := metric.GetMetricDescriptor().GetName()
-	metricType := se.metricTypeFromProto(metricName)
+	metricType := se.metricTypeFromProto(metric.GetMetricDescriptor().GetName())
 	metricLabelKeys := metric.GetMetricDescriptor().GetLabelKeys()
 	metricKind, valueType := protoMetricDescriptorTypeToMetricKind(metric)
 	labelKeys := make([]string, 0, len(metricLabelKeys))
@@ -246,6 +245,11 @@ func (se *statsExporter) protoMetricToTimeSeries(ctx context.Context, mappedRsc
 	}
 
 	for _, protoTimeSeries := range metric.Timeseries {
+		if len(protoTimeSeries.Points) == 0 {
+			// No points to send just move forward.
+			continue
+		}
+
 		sdPoints, err := se.protoTimeSeriesToMonitoringPoints(protoTimeSeries, metricKind)
 		if err != nil {
 			mb.recordDroppedTimeseries(1, err)
@@ -273,17 +277,21 @@ func (se *statsExporter) protoMetricToTimeSeries(ctx context.Context, mappedRsc
 }
 
 func labelsPerTimeSeries(defaults map[string]labelValue, labelKeys []string, labelValues []*metricspb.LabelValue) (map[string]string, error) {
+	if len(labelKeys) != len(labelValues) {
+		return nil, fmt.Errorf("length mismatch: len(labelKeys)=%d len(labelValues)=%d", len(labelKeys), len(labelValues))
+	}
+
+	if len(defaults)+len(labelKeys) == 0 {
+		// No labels for this metric
+		return nil, nil
+	}
+
 	labels := make(map[string]string)
 	// Fill in the defaults firstly, irrespective of if the labelKeys and labelValues are mismatched.
 	for key, label := range defaults {
 		labels[key] = label.val
 	}
 
-	// Perform this sanity check now.
-	if len(labelKeys) != len(labelValues) {
-		return labels, fmt.Errorf("length mismatch: len(labelKeys)=%d len(labelValues)=%d", len(labelKeys), len(labelValues))
-	}
-
 	for i, labelKey := range labelKeys {
 		labelValue := labelValues[i]
 		if !labelValue.GetHasValue() {
@@ -295,21 +303,15 @@ func labelsPerTimeSeries(defaults map[string]labelValue, labelKeys []string, lab
 	return labels, nil
 }
 
-func (se *statsExporter) createMetricDescriptorWithTimeout(ctx context.Context, metric *metricspb.Metric) error {
-	ctx, cancel := newContextWithTimeout(ctx, se.o.Timeout)
-	defer cancel()
-
-	return se.protoCreateMetricDescriptor(ctx, metric)
-}
-
-// createMetricDescriptor creates a metric descriptor from the OpenCensus proto metric
-// and then creates it remotely using Stackdriver's API.
-func (se *statsExporter) protoCreateMetricDescriptor(ctx context.Context, metric *metricspb.Metric) error {
+func (se *statsExporter) createMetricDescriptorFromMetricProto(ctx context.Context, metric *metricspb.Metric) error {
 	// Skip create metric descriptor if configured
 	if se.o.SkipCMD {
 		return nil
 	}
 
+	ctx, cancel := newContextWithTimeout(ctx, se.o.Timeout)
+	defer cancel()
+
 	se.protoMu.Lock()
 	defer se.protoMu.Unlock()
 
@@ -338,15 +340,15 @@ func (se *statsExporter) protoCreateMetricDescriptor(ctx context.Context, metric
 	return nil
 }
 
-func (se *statsExporter) protoTimeSeriesToMonitoringPoints(ts *metricspb.TimeSeries, metricKind googlemetricpb.MetricDescriptor_MetricKind) (sptl []*monitoringpb.Point, err error) {
+func (se *statsExporter) protoTimeSeriesToMonitoringPoints(ts *metricspb.TimeSeries, metricKind googlemetricpb.MetricDescriptor_MetricKind) ([]*monitoringpb.Point, error) {
+	sptl := make([]*monitoringpb.Point, 0, len(ts.Points))
 	for _, pt := range ts.Points {
 		// If we have a last value aggregation point i.e. MetricDescriptor_GAUGE
 		// StartTime should be nil.
 		startTime := ts.StartTimestamp
 		if metricKind == googlemetricpb.MetricDescriptor_GAUGE {
 			startTime = nil
 		}
-
 		spt, err := fromProtoPoint(startTime, pt)
 		if err != nil {
 			return nil, err

diff --git a/metrics_proto_test.go b/metrics_proto_test.go
@@ -269,7 +269,7 @@ func TestProtoMetricToCreateTimeSeriesRequest(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type:   "custom.googleapis.com/opencensus/with_metric_descriptor",
-								Labels: map[string]string{},
+								Labels: nil,
 							},
 							Resource: &monitoredrespb.MonitoredResource{
 								Type: "global",
@@ -455,7 +455,7 @@ func TestProtoMetricWithDifferentResource(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type:   "custom.googleapis.com/opencensus/with_container_resource",
-								Labels: map[string]string{},
+								Labels: nil,
 							},
 							Resource: &monitoredrespb.MonitoredResource{
 								Type: "k8s_container",
@@ -527,7 +527,7 @@ func TestProtoMetricWithDifferentResource(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type:   "custom.googleapis.com/opencensus/with_gce_resource",
-								Labels: map[string]string{},
+								Labels: nil,
 							},
 							Resource: &monitoredrespb.MonitoredResource{
 								Type: "gce_instance",

diff --git a/metrics_test.go b/metrics_test.go
@@ -145,7 +145,7 @@ func TestMetricToCreateTimeSeriesRequest(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type:   "custom.googleapis.com/opencensus/with_metric_descriptor",
-								Labels: map[string]string{},
+								Labels: nil,
 							},
 							Resource: &monitoredrespb.MonitoredResource{
 								Type: "global",
@@ -229,7 +229,7 @@ func TestMetricToCreateTimeSeriesRequest(t *testing.T) {
 						{
 							Metric: &googlemetricpb.Metric{
 								Type:   "custom.googleapis.com/opencensus/with_metric_descriptor",
-								Labels: map[string]string{},
+								Labels: nil,
 							},
 							Resource: &monitoredrespb.MonitoredResource{
 								Type: "global",

diff --git a/stats.go b/stats.go
@@ -597,11 +597,12 @@ func newLabelDescriptors(defaults map[string]labelValue, keys []tag.Key) []*labe
 }
 
 func (e *statsExporter) createMetricDescriptor(ctx context.Context, md *metric.MetricDescriptor) error {
+	ctx, cancel := newContextWithTimeout(ctx, e.o.Timeout)
+	defer cancel()
 	cmrdesc := &monitoringpb.CreateMetricDescriptorRequest{
 		Name:             fmt.Sprintf("projects/%s", e.o.ProjectID),
 		MetricDescriptor: md,
 	}
-
 	_, err := createMetricDescriptor(ctx, e.c, cmrdesc)
 	return err
 }