Repository: census-ecosystem/opencensus-go-exporter-stackdriver
Pull request #184 (2019-08-29 18:00:44)

Remove mandatory opencensus_task label from node and respect metric prefix


Patch:
diff --git a/metrics.go b/metrics.go
index 4fda8d4..e574e0e 100644
--- a/metrics.go
+++ b/metrics.go
@@ -135,7 +135,7 @@ func (se *statsExporter) metricToMpbTs(ctx context.Context, metric *metricdata.M
 	resource := se.metricRscToMpbRsc(metric.Resource)
 
 	metricName := metric.Descriptor.Name
-	metricType, _ := se.metricTypeFromProto(metricName)
+	metricType := se.metricTypeFromProto(metricName)
 	metricLabelKeys := metric.Descriptor.LabelKeys
 	metricKind, _ := metricDescriptorTypeToMetricKind(metric)
 
@@ -238,7 +238,7 @@ func (se *statsExporter) metricToMpbMetricDescriptor(metric *metricdata.Metric)
 		return nil, errNilMetricOrMetricDescriptor
 	}
 
-	metricType, _ := se.metricTypeFromProto(metric.Descriptor.Name)
+	metricType := se.metricTypeFromProto(metric.Descriptor.Name)
 	displayName := se.displayName(metric.Descriptor.Name)
 	metricKind, valueType := metricDescriptorTypeToMetricKind(metric)
 
diff --git a/metrics_proto.go b/metrics_proto.go
index afe5465..3a6e288 100644
--- a/metrics_proto.go
+++ b/metrics_proto.go
@@ -49,6 +49,7 @@ var percentileLabelKey = &metricspb.LabelKey{
 	Description: "the value at a given percentile of a distribution",
 }
 var globalResource = &resource.Resource{Type: "global"}
+var domains = []string{"googleapis.com", "kubernetes.io", "istio.io"}
 
 type metricProtoPayload struct {
 	node             *commonpb.Node
@@ -75,17 +76,11 @@ func (se *statsExporter) ExportMetricsProto(ctx context.Context, node *commonpb.
 		return errNilMetricOrMetricDescriptor
 	}
 
-	additionalLabels := se.defaultLabels
-	if additionalLabels == nil {
-		// additionalLabels must be stateless because each node is different
-		additionalLabels = getDefaultLabelsFromNode(node)
-	}
-
 	for _, metric := range metrics {
 		if metric.GetMetricDescriptor().GetType() == metricspb.MetricDescriptor_SUMMARY {
-			se.addPayload(node, rsc, additionalLabels, se.convertSummaryMetrics(metric)...)
+			se.addPayload(node, rsc, se.defaultLabels, se.convertSummaryMetrics(metric)...)
 		} else {
-			se.addPayload(node, rsc, additionalLabels, metric)
+			se.addPayload(node, rsc, se.defaultLabels, metric)
 		}
 	}
 
@@ -102,12 +97,6 @@ func (se *statsExporter) ExportMetricsProtoSync(ctx context.Context, node *commo
 	// Caches the resources seen so far
 	seenResources := make(map[*resourcepb.Resource]*monitoredrespb.MonitoredResource)
 
-	additionalLabels := se.defaultLabels
-	if additionalLabels == nil {
-		// additionalLabels must be stateless because each node is different
-		additionalLabels = getDefaultLabelsFromNode(node)
-	}
-
 	ctx, cancel := se.o.newContextWithTimeout()
 	defer cancel()
 
@@ -123,14 +112,14 @@ func (se *statsExporter) ExportMetricsProtoSync(ctx context.Context, node *commo
 		if metric.GetMetricDescriptor().GetType() == metricspb.MetricDescriptor_SUMMARY {
 			summaryMtcs := se.convertSummaryMetrics(metric)
 			for _, summaryMtc := range summaryMtcs {
-				if tss, err := se.protoMetricToTimeSeries(ctx, mappedRsc, summaryMtc, additionalLabels); err == nil {
+				if tss, err := se.protoMetricToTimeSeries(ctx, mappedRsc, summaryMtc, se.defaultLabels); err == nil {
 					allTss = append(tss, tss...)
 				} else {
 					allErrs = append(allErrs, err)
 				}
 			}
 		} else {
-			if tss, err := se.protoMetricToTimeSeries(ctx, mappedRsc, metric, additionalLabels); err == nil {
+			if tss, err := se.protoMetricToTimeSeries(ctx, mappedRsc, metric, se.defaultLabels); err == nil {
 				allTss = append(allTss, tss...)
 			} else {
 				allErrs = append(allErrs, err)
@@ -450,7 +439,7 @@ func (se *statsExporter) protoMetricToTimeSeries(ctx context.Context, mappedRsc
 	}
 
 	metricName := metric.GetMetricDescriptor().GetName()
-	metricType, _ := se.metricTypeFromProto(metricName)
+	metricType := se.metricTypeFromProto(metricName)
 	metricLabelKeys := metric.GetMetricDescriptor().GetLabelKeys()
 	metricKind, valueType := protoMetricDescriptorTypeToMetricKind(metric)
 	labelKeys := make([]string, len(metricLabelKeys))
@@ -594,7 +583,7 @@ func (se *statsExporter) protoToMonitoringMetricDescriptor(metric *metricspb.Met
 	metricName := md.GetName()
 	unit := md.GetUnit()
 	description := md.GetDescription()
-	metricType, _ := se.metricTypeFromProto(metricName)
+	metricType := se.metricTypeFromProto(metricName)
 	displayName := se.displayName(metricName)
 	metricKind, valueType := protoMetricDescriptorTypeToMetricKind(metric)
 
@@ -635,10 +624,26 @@ func labelDescriptorsFromProto(defaults map[string]labelValue, protoLabelKeys []
 	return labelDescriptors
 }
 
-func (se *statsExporter) metricTypeFromProto(name string) (string, bool) {
-	// TODO: (@odeke-em) support non-"custom.googleapis.com" metrics names.
-	name = path.Join("custom.googleapis.com", "opencensus", name)
-	return name, true
+func (se *statsExporter) metricTypeFromProto(name string) string {
+	prefix := se.o.MetricPrefix
+	if prefix != "" {
+		name = prefix + name
+	}
+	if !hasDomain(name) {
+		// Still needed because the name may or may not have a "/" at the beginning.
+		name = path.Join(defaultDomain, name)
+	}
+	return name
+}
+
+// hasDomain checks if the metric name already has a domain in it.
+func hasDomain(name string) bool {
+	for _, domain := range domains {
+		if strings.Contains(name, domain) {
+			return true
+		}
+	}
+	return false
 }
 
 func fromProtoPoint(startTime *timestamp.Timestamp, pt *metricspb.Point) (*monitoringpb.Point, error) {
@@ -776,16 +781,6 @@ func protoMetricDescriptorTypeToMetricKind(m *metricspb.Metric) (googlemetricpb.
 	}
 }
 
-func getDefaultLabelsFromNode(node *commonpb.Node) map[string]labelValue {
-	taskValue := fmt.Sprintf("%s-%d@%s", strings.ToLower(node.LibraryInfo.GetLanguage().String()), node.Identifier.Pid, node.Identifier.HostName)
-	return map[string]labelValue{
-		sanitize(opencensusTaskKey): {
-			val:  taskValue,
-			desc: opencensusTaskDescription,
-		},
-	}
-}
-
 // combineErrors converts a list of errors into one error.
 func combineErrors(errs []error) error {
 	numErrors := len(errs)
diff --git a/stackdriver.go b/stackdriver.go
index a96931e..bd20e31 100644
--- a/stackdriver.go
+++ b/stackdriver.go
@@ -54,6 +54,7 @@ import (
 	"log"
 	"os"
 	"path"
+	"strings"
 	"time"
 
 	metadataapi "cloud.google.com/go/compute/metadata"
@@ -263,6 +264,8 @@ type Options struct {
 
 const defaultTimeout = 5 * time.Second
 
+var defaultDomain = path.Join("custom.googleapis.com", "opencensus")
+
 // Exporter is a stats and trace exporter that uploads data to Stackdriver.
 //
 // You can create a single Exporter and register it as both a trace exporter
@@ -336,6 +339,9 @@ func NewExporter(o Options) (*Exporter, error) {
 
 		o.Resource = o.MapResource(res)
 	}
+	if o.MetricPrefix != "" && !strings.HasSuffix(o.MetricPrefix, "/") {
+		o.MetricPrefix = o.MetricPrefix + "/"
+	}
 
 	se, err := newStatsExporter(o)
 	if err != nil {


Test patch:
diff --git a/metrics_proto_test.go b/metrics_proto_test.go
index 5e377a9..b64535d 100644
--- a/metrics_proto_test.go
+++ b/metrics_proto_test.go
@@ -16,7 +16,6 @@ package stackdriver
 
 import (
 	"context"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -29,7 +28,6 @@ import (
 	monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
 	monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
 
-	commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1"
 	metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1"
 	"github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/google/go-cmp/cmp"
@@ -632,45 +630,6 @@ func TestCombineTimeSeriesAndDeduplication(t *testing.T) {
 	}
 }
 
-func TestNodeToDefaultLabels(t *testing.T) {
-	tests := []struct {
-		in   *commonpb.Node
-		want map[string]labelValue
-	}{
-		{
-			in: &commonpb.Node{
-				Identifier:  &commonpb.ProcessIdentifier{HostName: "host1", Pid: 8081},
-				LibraryInfo: &commonpb.LibraryInfo{Language: commonpb.LibraryInfo_JAVA},
-			},
-			want: map[string]labelValue{
-				"opencensus_task": {
-					val:  "java-8081@host1",
-					desc: "Opencensus task identifier",
-				},
-			},
-		},
-		{
-			in: &commonpb.Node{
-				Identifier:  &commonpb.ProcessIdentifier{HostName: "host2", Pid: 9090},
-				LibraryInfo: &commonpb.LibraryInfo{Language: commonpb.LibraryInfo_PYTHON},
-			},
-			want: map[string]labelValue{
-				"opencensus_task": {
-					val:  "python-9090@host2",
-					desc: "Opencensus task identifier",
-				},
-			},
-		},
-	}
-
-	for i, tt := range tests {
-		got := getDefaultLabelsFromNode(tt.in)
-		if !reflect.DeepEqual(got, tt.want) {
-			t.Fatalf("Test %d failed. Default labels mismatch. Want %v\nGot %v\n", i, tt.want, got)
-		}
-	}
-}
-
 func TestConvertSummaryMetrics(t *testing.T) {
 	startTimestamp := &timestamp.Timestamp{
 		Seconds: 1543160298,
@@ -790,6 +749,47 @@ func TestConvertSummaryMetrics(t *testing.T) {
 	}
 }
 
+func TestMetricPrefix(t *testing.T) {
+	tests := []struct {
+		name          string
+		in            string
+		want          string
+		statsExporter *statsExporter
+	}{
+		{
+			name: "No prefix and metric name has a kubernetes domain",
+			in:   "kubernetes.io/container/memory/limit_bytes",
+			statsExporter: &statsExporter{
+				o: Options{ProjectID: "foo"},
+			},
+			want: "kubernetes.io/container/memory/limit_bytes",
+		},
+		{
+			name: "Has a prefix but prefix doesn't have a domain",
+			in:   "my_metric",
+			statsExporter: &statsExporter{
+				o: Options{ProjectID: "foo", MetricPrefix: "prefix/"},
+			},
+			want: "custom.googleapis.com/opencensus/prefix/my_metric",
+		},
+		{
+			name: "Has a prefix and prefix has a domain",
+			in:   "my_metric",
+			statsExporter: &statsExporter{
+				o: Options{ProjectID: "foo", MetricPrefix: "appengine.googleapis.com/"},
+			},
+			want: "appengine.googleapis.com/my_metric",
+		},
+	}
+
+	for _, tt := range tests {
+		got := tt.statsExporter.metricTypeFromProto(tt.in)
+		if !cmp.Equal(got, tt.want) {
+			t.Fatalf("mismatch metric names for test %v:\n  got=%v\n want=%v\n", tt.name, got, tt.want)
+		}
+	}
+}
+
 func makeInt64Ts(val int64, label string, start, end *timestamp.Timestamp) *metricspb.TimeSeries {
 	ts := &metricspb.TimeSeries{
 		StartTimestamp: start,