Convert RPC metrics to use tags

rpcmetrics/metrics.go

@@ -34,29 +34,26 @@ const (
 // Metrics is a collection of metrics for an endpoint describing
 // throughput, success, errors, and performance.
 type Metrics struct {
-	// Requests is a counter of the total number of successes + failures.
-	Requests metrics.Counter `metric:"requests"`
-
 	// Success is a counter of the total number of successes.
-	Success metrics.Counter `metric:"success"`
+	Success metrics.Counter `metric:"requests" tags:"error=false"`
 
 	// Failures is a counter of the number of times any failure has been observed.
-	Failures metrics.Counter `metric:"failures"`
+	Failures metrics.Counter `metric:"requests" tags:"error=true"`
 
 	// RequestLatency is a histogram of the latency of requests.
 	RequestLatency metrics.Timer `metric:"request_latency"`
 
 	// HTTPStatusCode2xx is a counter of the total number of requests with HTTP status code 200-299
-	HTTPStatusCode2xx metrics.Counter `metric:"http_status_code_2xx"`
+	HTTPStatusCode2xx metrics.Counter `metric:"http_requests" tags:"status_code=2xx"`
 
 	// HTTPStatusCode3xx is a counter of the total number of requests with HTTP status code 300-399
-	HTTPStatusCode3xx metrics.Counter `metric:"http_status_code_3xx"`
+	HTTPStatusCode3xx metrics.Counter `metric:"http_requests" tags:"status_code=3xx"`
 
 	// HTTPStatusCode4xx is a counter of the total number of requests with HTTP status code 400-499
-	HTTPStatusCode4xx metrics.Counter `metric:"http_status_code_4xx"`
+	HTTPStatusCode4xx metrics.Counter `metric:"http_requests" tags:"status_code=4xx"`
 
 	// HTTPStatusCode5xx is a counter of the total number of requests with HTTP status code 500-599
-	HTTPStatusCode5xx metrics.Counter `metric:"http_status_code_5xx"`
+	HTTPStatusCode5xx metrics.Counter `metric:"http_requests" tags:"status_code=5xx"`
 }
 
 func (m *Metrics) recordHTTPStatusCode(statusCode uint16) {

rpcmetrics/metrics_test.go

@@ -28,10 +28,17 @@ import (
 	"github.com/uber/jaeger-lib/metrics/testutils"
 )
 
-func endpointTags(endpoint string) map[string]string {
-	return map[string]string{
-		"endpoint": endpoint,
+// E.g. tags("key", "value", "key", "value")
+func tags(kv ...string) map[string]string {
+	m := make(map[string]string)
+	for i := 0; i < len(kv)-1; i += 2 {
+		m[kv[i]] = kv[i+1]
 	}
+	return m
+}
+
+func endpointTags(endpoint string, kv ...string) map[string]string {
+	return tags(append([]string{"endpoint", endpoint}, kv...)...)
 }
 
 func TestMetricsByEndpoint(t *testing.T) {
@@ -49,12 +56,12 @@ func TestMetricsByEndpoint(t *testing.T) {
 	m5 := mbe.get("overflow2")
 
 	for _, m := range []*Metrics{m1, m2, m2a, m3, m4, m5} {
-		m.Requests.Inc(1)
+		m.Success.Inc(1)
 	}
 
 	testutils.AssertCounterMetrics(t, met,
-		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("abc1"), Value: 3},
-		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("abc3"), Value: 1},
-		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("other"), Value: 2},
+		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("abc1", "error", "false"), Value: 3},
+		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("abc3", "error", "false"), Value: 1},
+		testutils.ExpectedMetric{Name: "requests", Tags: endpointTags("other", "error", "false"), Value: 2},
 	)
 }

rpcmetrics/observer.go

@@ -151,7 +151,6 @@ func (so *SpanObserver) OnFinish(options opentracing.FinishOptions) {
 	}
 
 	mets := so.metricsByEndpoint.get(so.operationName)
-	mets.Requests.Inc(1)
 	if so.err {
 		mets.Failures.Inc(1)
 	} else {

rpcmetrics/observer_test.go

@@ -52,13 +52,11 @@ func ExampleObserver() {
 	span.Finish()
 
 	c, _ := metricsFactory.Snapshot()
-	fmt.Printf("requests: %d\n", c["requests|endpoint=test"])
-	fmt.Printf("success: %d\n", c["success|endpoint=test"])
-	fmt.Printf("errors: %d\n", c["errors|endpoint=test"])
+	fmt.Printf("requests (success): %d\n", c["requests|endpoint=test|error=false"])
+	fmt.Printf("requests (failure): %d\n", c["requests|endpoint=test|error=true"])
 	// Output:
-	// requests: 1
-	// success: 1
-	// errors: 0
+	// requests (success): 1
+	// requests (failure): 0
 }
 
 type testTracer struct {
@@ -115,14 +113,10 @@ func TestObserver(t *testing.T) {
 
 		u.AssertCounterMetrics(t,
 			testTracer.metrics,
-			u.ExpectedMetric{Name: "requests", Tags: endpointTags("local-span"), Value: 0},
-			u.ExpectedMetric{Name: "success", Tags: endpointTags("local-span"), Value: 0},
-			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user"), Value: 1},
-			u.ExpectedMetric{Name: "success", Tags: endpointTags("get-user"), Value: 1},
-			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user-override"), Value: 1},
-			u.ExpectedMetric{Name: "success", Tags: endpointTags("get-user-override"), Value: 1},
-			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user-client"), Value: 0},
-			u.ExpectedMetric{Name: "success", Tags: endpointTags("get-user-client"), Value: 0},
+			u.ExpectedMetric{Name: "requests", Tags: endpointTags("local-span", "error", "false"), Value: 0},
+			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user", "error", "false"), Value: 1},
+			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user-override", "error", "false"), Value: 1},
+			u.ExpectedMetric{Name: "requests", Tags: endpointTags("get-user-client", "error", "false"), Value: 0},
 		)
 		// TODO something wrong with string generation, .P99 should not be appended to the tag
 		// as a result we cannot use u.AssertGaugeMetrics
@@ -140,16 +134,13 @@ func TestTags(t *testing.T) {
 
 	testCases := []tagTestCase{
 		{key: "something", value: 42, metrics: []u.ExpectedMetric{
-			{Name: "success", Value: 1},
-			{Name: "requests", Value: 1},
+			{Name: "requests", Value: 1, Tags: tags("error", "false")},
 		}},
 		{key: "error", value: true, metrics: []u.ExpectedMetric{
-			{Name: "failures", Value: 1},
-			{Name: "requests", Value: 1},
+			{Name: "requests", Value: 1, Tags: tags("error", "true")},
 		}},
 		{key: "error", value: "true", metrics: []u.ExpectedMetric{
-			{Name: "failures", Value: 1},
-			{Name: "requests", Value: 1},
+			{Name: "requests", Value: 1, Tags: tags("error", "true")},
 		}},
 	}
 
@@ -162,8 +153,7 @@ func TestTags(t *testing.T) {
 		for _, v := range values {
 			testCases = append(testCases, tagTestCase{
 				key: "http.status_code", value: v, metrics: []u.ExpectedMetric{
-					{Name: "requests", Value: 1},
-					{Name: fmt.Sprintf("http_status_code_%dxx", i), Value: 1},
+					{Name: "http_requests", Value: 1, Tags: tags("status_code", fmt.Sprintf("%dxx", i))},
 				},
 			})
 		}
@@ -172,7 +162,7 @@ func TestTags(t *testing.T) {
 	for _, tc := range testCases {
 		testCase := tc // capture loop var
 		for i := range testCase.metrics {
-			testCase.metrics[i].Tags = endpointTags("span")
+			testCase.metrics[i].Tags["endpoint"] = "span"
 		}
 		t.Run(fmt.Sprintf("%s-%v", testCase.key, testCase.value), func(t *testing.T) {
 			withTestTracer(func(testTracer *testTracer) {