Repository: census-ecosystem/opencensus-go-exporter-stackdriver (go)

Issue:
GetMetricType and MetricPrefix do not work as their descriptions say in new reporting pipeline
**Is your feature request related to a problem? Please describe.**
Description of `MetricPrefix`:
```
// MetricPrefix overrides the prefix of a Stackdriver metric display names.
// Optional. If unset defaults to "OpenCensus/".
// Deprecated: Provide GetMetricDisplayName to change the display name of
// the metric.
// If GetMetricDisplayName is non-nil, this option is ignored.
```
It's about display name, however it is used as metric prefix [here](https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver/blob/61e3caa20a46015704f45956535fe83133eaf62e/metrics_proto.go#L412)

Description of `GetMetricType`:
```
// GetMetricType allows customizing the metric type for the given view.
// By default, it will be:
//   "custom.googleapis.com/opencensus/" + view.Name
//
// See: https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.metricDescriptors#MetricDescriptor
```

However it is not used in `metrics.go`.

**Describe the solution you'd like**
Use `GetMetricType` instead of `MetricPrefix` to determine metric prefix.

**Describe alternatives you've considered**
My proposal is what is used in the old pipeline: https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver/blob/61e3caa20a46015704f45956535fe83133eaf62e/stats.go#L564
I think it is a bug. Unless we want to change the behavior, then no alternatives are considered.


Pull request:
Add GetMetricPrefix func as an option to support dynamic custom metric prefix
Fix #237.

* Add `GetMetricPrefix` option to decide metric prefix based on metric name.
* Add "knative.dev" to supported domains.
* Update `MetricPrefix` description and stop using it for display name.

Patch:
diff --git a/metrics_proto.go b/metrics_proto.go
index 5fe4b39..bcc1f0e 100644
--- a/metrics_proto.go
+++ b/metrics_proto.go
@@ -45,7 +45,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"}
+var domains = []string{"googleapis.com", "kubernetes.io", "istio.io", "knative.dev"}
 
 // PushMetricsProto exports OpenCensus Metrics Proto to Stackdriver Monitoring synchronously,
 // without de-duping or adding proto metrics to the bundler.
@@ -410,8 +410,11 @@ func labelDescriptorsFromProto(defaults map[string]labelValue, protoLabelKeys []
 
 func (se *statsExporter) metricTypeFromProto(name string) string {
 	prefix := se.o.MetricPrefix
+	if se.o.GetMetricPrefix != nil {
+		prefix = se.o.GetMetricPrefix(name)
+	}
 	if prefix != "" {
-		name = prefix + name
+		name = path.Join(prefix, name)
 	}
 	if !hasDomain(name) {
 		// Still needed because the name may or may not have a "/" at the beginning.
diff --git a/stackdriver.go b/stackdriver.go
index ff2aee7..fafd06c 100644
--- a/stackdriver.go
+++ b/stackdriver.go
@@ -186,11 +186,9 @@ type Options struct {
 	// conversions from auto-detected resources to well-known Stackdriver monitored resources.
 	MapResource func(*resource.Resource) *monitoredrespb.MonitoredResource
 
-	// MetricPrefix overrides the prefix of a Stackdriver metric display names.
-	// Optional. If unset defaults to "OpenCensus/".
-	// Deprecated: Provide GetMetricDisplayName to change the display name of
-	// the metric.
-	// If GetMetricDisplayName is non-nil, this option is ignored.
+	// MetricPrefix overrides the prefix of a Stackdriver metric names.
+	// Optional. If unset defaults to "custom.googleapis.com/opencensus/".
+	// If GetMetricPrefix is non-nil, this option is ignored.
 	MetricPrefix string
 
 	// GetMetricDisplayName allows customizing the display name for the metric
@@ -203,8 +201,16 @@ type Options struct {
 	//   "custom.googleapis.com/opencensus/" + view.Name
 	//
 	// See: https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.metricDescriptors#MetricDescriptor
+	// Depreacted. Use GetMetricPrefix instead.
 	GetMetricType func(view *view.View) string
 
+	// GetMetricPrefix allows customizing the metric prefix for the given metric name.
+	// If it is not set, MetricPrefix is used. If MetricPrefix is not set, it defaults to:
+	//   "custom.googleapis.com/opencensus/"
+	//
+	// See: https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.metricDescriptors#MetricDescriptor
+	GetMetricPrefix func(name string) string
+
 	// DefaultTraceAttributes will be appended to every span that is exported to
 	// Stackdriver Trace.
 	DefaultTraceAttributes map[string]interface{}
diff --git a/stats.go b/stats.go
index 1b8164f..e0a02ca 100644
--- a/stats.go
+++ b/stats.go
@@ -365,11 +365,7 @@ func (e *statsExporter) createMetricDescriptorFromView(ctx context.Context, v *v
 }
 
 func (e *statsExporter) displayName(suffix string) string {
-	displayNamePrefix := defaultDisplayNamePrefix
-	if e.o.MetricPrefix != "" {
-		displayNamePrefix = e.o.MetricPrefix
-	}
-	return path.Join(displayNamePrefix, suffix)
+	return path.Join(defaultDisplayNamePrefix, suffix)
 }
 
 func (e *statsExporter) combineTimeSeriesToCreateTimeSeriesRequest(ts []*monitoringpb.TimeSeries) (ctsreql []*monitoringpb.CreateTimeSeriesRequest) {


Test patch:
diff --git a/metrics_proto_api_test.go b/metrics_proto_api_test.go
index 0cf74d4..7e29d2e 100644
--- a/metrics_proto_api_test.go
+++ b/metrics_proto_api_test.go
@@ -94,7 +94,6 @@ func TestMetricsWithPrefix(t *testing.T) {
 	metricName := "ocagent.io"
 	saveMetricType := tc.outTSR[0].TimeSeries[0].Metric.Type
 	saveMDRName := tc.outMDR[0].MetricDescriptor.Name
-	saveMDRDispName := tc.outMDR[0].MetricDescriptor.DisplayName
 
 	for _, prefix := range prefixes {
 		opts := defaultOpts
@@ -104,7 +103,6 @@ func TestMetricsWithPrefix(t *testing.T) {
 		mt := strings.Replace(saveMetricType, metricName, (prefix + metricName), -1)
 		tc.outMDR[0].MetricDescriptor.Name = strings.Replace(saveMDRName, metricName, (prefix + metricName), -1)
 		tc.outMDR[0].MetricDescriptor.Type = mt
-		tc.outMDR[0].MetricDescriptor.DisplayName = strings.Replace(saveMDRDispName, "OpenCensus/"+metricName, (prefix + metricName), -1)
 
 		tc.outTSR[0].TimeSeries[0].Metric.Type = mt
 
@@ -125,7 +123,6 @@ func TestMetricsWithPrefixWithDomain(t *testing.T) {
 	metricName := "ocagent.io"
 	saveMetricType := strings.Replace(tc.outTSR[0].TimeSeries[0].Metric.Type, "custom.googleapis.com/opencensus/", "", -1)
 	saveMDRName := strings.Replace(tc.outMDR[0].MetricDescriptor.Name, "custom.googleapis.com/opencensus/", "", -1)
-	saveMDRDispName := tc.outMDR[0].MetricDescriptor.DisplayName
 
 	for _, prefix := range prefixes {
 		opts := defaultOpts
@@ -135,7 +132,6 @@ func TestMetricsWithPrefixWithDomain(t *testing.T) {
 		mt := strings.Replace(saveMetricType, metricName, (prefix + metricName), -1)
 		tc.outMDR[0].MetricDescriptor.Name = strings.Replace(saveMDRName, metricName, (prefix + metricName), -1)
 		tc.outMDR[0].MetricDescriptor.Type = mt
-		tc.outMDR[0].MetricDescriptor.DisplayName = strings.Replace(saveMDRDispName, "OpenCensus/"+metricName, (prefix + metricName), -1)
 
 		tc.outTSR[0].TimeSeries[0].Metric.Type = mt
 
diff --git a/metrics_proto_test.go b/metrics_proto_test.go
index 7d21c66..f69a9f4 100644
--- a/metrics_proto_test.go
+++ b/metrics_proto_test.go
@@ -1004,6 +1004,14 @@ func TestMetricPrefix(t *testing.T) {
 			},
 			want: "custom.googleapis.com/opencensus/prefix/my_metric",
 		},
+		{
+			name: "Has a prefix without `/` ending 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",
@@ -1012,6 +1020,43 @@ func TestMetricPrefix(t *testing.T) {
 			},
 			want: "appengine.googleapis.com/my_metric",
 		},
+		{
+			name: "Has a GetMetricPrefix func but result doesn't have a domain",
+			in:   "my_metric",
+			statsExporter: &statsExporter{
+				o: Options{
+					ProjectID: "foo",
+					GetMetricPrefix: func(name string) string {
+						return "prefix"
+					}},
+			},
+			want: "custom.googleapis.com/opencensus/prefix/my_metric",
+		},
+		{
+			name: "Has a GetMetricPrefix func and result has a domain",
+			in:   "my_metric",
+			statsExporter: &statsExporter{
+				o: Options{
+					ProjectID: "foo",
+					GetMetricPrefix: func(name string) string {
+						return "knative.dev/serving"
+					}},
+			},
+			want: "knative.dev/serving/my_metric",
+		},
+		{
+			name: "Has both a prefix and GetMetricPrefix func",
+			in:   "my_metric",
+			statsExporter: &statsExporter{
+				o: Options{
+					ProjectID:    "foo",
+					MetricPrefix: "appengine.googleapis.com/",
+					GetMetricPrefix: func(name string) string {
+						return "knative.dev/serving"
+					}},
+			},
+			want: "knative.dev/serving/my_metric",
+		},
 	}
 
 	for _, tt := range tests {