Repository: census-instrumentation/opencensus-go
Pull request #1157 (2019-08-29 06:37:36)

change unit to dimensionless when aggregation is count.
The unit for Aggregation Count should always be Dimensionless.
For example if latency measures (with unit "ms")  is used into two views
- View1 : Aggregation Sum
- View2: Aggregation Count
then the unit "ms" for View2 doesn't make sense. It should be Dimensionless. 


Patch:
diff --git a/stats/view/view_to_metric.go b/stats/view/view_to_metric.go
index f67b5c4..293c164 100644
--- a/stats/view/view_to_metric.go
+++ b/stats/view/view_to_metric.go
@@ -85,12 +85,21 @@ func viewToMetricDescriptor(v *View) *metricdata.Descriptor {
 	return &metricdata.Descriptor{
 		Name:        v.Name,
 		Description: v.Description,
-		Unit:        getUnit(v.Measure.Unit()),
+		Unit:        convertUnit(v),
 		Type:        getType(v),
 		LabelKeys:   getLabelKeys(v),
 	}
 }
 
+func convertUnit(v *View) metricdata.Unit {
+	switch v.Aggregation.Type {
+	case AggTypeCount:
+		return metricdata.UnitDimensionless
+	default:
+		return getUnit(v.Measure.Unit())
+	}
+}
+
 func toLabelValues(row *Row, expectedKeys []metricdata.LabelKey) []metricdata.LabelValue {
 	labelValues := []metricdata.LabelValue{}
 	tagMap := make(map[string]string)


Test patch:
diff --git a/stats/view/view_to_metric_test.go b/stats/view/view_to_metric_test.go
index da41354..e4a0b27 100644
--- a/stats/view/view_to_metric_test.go
+++ b/stats/view/view_to_metric_test.go
@@ -457,6 +457,65 @@ func Test_ViewToMetric(t *testing.T) {
 	}
 }
 
+// Test to verify that a metric converted from a view with Aggregation Count should always
+// have Dimensionless unit.
+func TestUnitConversionForAggCount(t *testing.T) {
+	startTime := time.Now().Add(-time.Duration(60 * time.Second))
+	now := time.Now()
+	tests := []*struct {
+		name     string
+		vi       *viewInternal
+		v        *View
+		wantUnit metricdata.Unit
+	}{
+		{
+			name: "View with Count Aggregation on Latency measurement",
+			v: &View{
+				Name:        "request_count1",
+				Measure:     stats.Int64("request_latency", "", stats.UnitMilliseconds),
+				Aggregation: aggCnt,
+			},
+			wantUnit: metricdata.UnitDimensionless,
+		},
+		{
+			name: "View with Count Aggregation on bytes measurement",
+			v: &View{
+				Name:        "request_count2",
+				Measure:     stats.Int64("request_bytes", "", stats.UnitBytes),
+				Aggregation: aggCnt,
+			},
+			wantUnit: metricdata.UnitDimensionless,
+		},
+		{
+			name: "View with aggregation other than Count Aggregation on Latency measurement",
+			v: &View{
+				Name:        "request_latency",
+				Measure:     stats.Int64("request_latency", "", stats.UnitMilliseconds),
+				Aggregation: aggSum,
+			},
+			wantUnit: metricdata.UnitMilliseconds,
+		},
+	}
+	var err error
+	for _, tc := range tests {
+		tc.vi, err = defaultWorker.tryRegisterView(tc.v)
+		if err != nil {
+			t.Fatalf("error registering view: %v, err: %v\n", tc.v, err)
+		}
+		tc.vi.clearRows()
+		tc.vi.subscribe()
+	}
+
+	for _, tc := range tests {
+		tc.vi.addSample(tag.FromContext(context.Background()), 5.0, nil, now)
+		gotMetric := viewToMetric(tc.vi, now, startTime)
+		gotUnit := gotMetric.Descriptor.Unit
+		if !cmp.Equal(gotUnit, tc.wantUnit) {
+			t.Errorf("Verify Unit: %s: Got:%v Want:%v", tc.name, gotUnit, tc.wantUnit)
+		}
+	}
+}
+
 func serializeAsJSON(v interface{}) string {
 	blob, _ := json.MarshalIndent(v, "", "  ")
 	return string(blob)