Consolidate query metrics and include result tag

Signed-off-by: Gary Brown <[email protected]>

Author: objectiser

Diff for: storage/spanstore/metrics/decorator.go

@@ -34,16 +34,15 @@ type ReadMetricsDecorator struct {
 }
 
 type queryMetrics struct {
-	Errors     metrics.Counter `metric:"errors"`
-	Attempts   metrics.Counter `metric:"attempts"`
-	Successes  metrics.Counter `metric:"successes"`
-	Responses  metrics.Timer   `metric:"responses"` //used as a histogram, not necessary for GetTrace
-	ErrLatency metrics.Timer   `metric:"errLatency"`
-	OKLatency  metrics.Timer   `metric:"okLatency"`
+	Errors     metrics.Counter
+	Attempts   metrics.Counter
+	Successes  metrics.Counter
+	Responses  metrics.Timer //used as a histogram, not necessary for GetTrace
+	ErrLatency metrics.Timer
+	OKLatency  metrics.Timer
 }
 
 func (q *queryMetrics) emit(err error, latency time.Duration, responses int) {
-	q.Attempts.Inc(1)
 	if err != nil {
 		q.Errors.Inc(1)
 		q.ErrLatency.Record(latency)
@@ -66,9 +65,14 @@ func NewReadMetricsDecorator(spanReader spanstore.Reader, metricsFactory metrics
 }
 
 func buildQueryMetrics(namespace string, metricsFactory metrics.Factory) *queryMetrics {
-	qMetrics := &queryMetrics{}
 	scoped := metricsFactory.Namespace(namespace, nil)
-	metrics.Init(qMetrics, scoped, nil)
+	qMetrics := &queryMetrics{
+		Errors:     scoped.Counter("requests", map[string]string{"result": "err"}),
+		Successes:  scoped.Counter("requests", map[string]string{"result": "ok"}),
+		Responses:  scoped.Timer("responses", nil),
+		ErrLatency: scoped.Timer("latency", map[string]string{"result": "err"}),
+		OKLatency:  scoped.Timer("latency", map[string]string{"result": "ok"}),
+	}
 	return qMetrics
 }
 

Diff for: storage/spanstore/metrics/decorator_test.go

@@ -43,27 +43,23 @@ func TestSuccessfulUnderlyingCalls(t *testing.T) {
 	mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{})
 	counters, gauges := mf.Snapshot()
 	expecteds := map[string]int64{
-		"get_operations.attempts":  1,
-		"get_operations.successes": 1,
-		"get_operations.errors":    0,
-		"get_trace.attempts":       1,
-		"get_trace.successes":      1,
-		"get_trace.errors":         0,
-		"find_traces.attempts":     1,
-		"find_traces.successes":    1,
-		"find_traces.errors":       0,
-		"get_services.attempts":    1,
-		"get_services.successes":   1,
-		"get_services.errors":      0,
+		"get_operations.requests|result=ok":  1,
+		"get_operations.requests|result=err": 0,
+		"get_trace.requests|result=ok":       1,
+		"get_trace.requests|result=err":      0,
+		"find_traces.requests|result=ok":     1,
+		"find_traces.requests|result=err":    0,
+		"get_services.requests|result=ok":    1,
+		"get_services.requests|result=err":   0,
 	}
 
 	existingKeys := []string{
-		"get_operations.okLatency.P50",
+		"get_operations.latency|result=ok.P50",
 		"get_trace.responses.P50",
-		"find_traces.okLatency.P50", // this is not exhaustive
+		"find_traces.latency|result=ok.P50", // this is not exhaustive
 	}
 	nonExistentKeys := []string{
-		"get_operations.errLatency.P50",
+		"get_operations.latency|result=err.P50",
 	}
 
 	checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys)
@@ -100,28 +96,24 @@ func TestFailingUnderlyingCalls(t *testing.T) {
 	mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{})
 	counters, gauges := mf.Snapshot()
 	expecteds := map[string]int64{
-		"get_operations.attempts":  1,
-		"get_operations.successes": 0,
-		"get_operations.errors":    1,
-		"get_trace.attempts":       1,
-		"get_trace.successes":      0,
-		"get_trace.errors":         1,
-		"find_traces.attempts":     1,
-		"find_traces.successes":    0,
-		"find_traces.errors":       1,
-		"get_services.attempts":    1,
-		"get_services.successes":   0,
-		"get_services.errors":      1,
+		"get_operations.requests|result=ok":  0,
+		"get_operations.requests|result=err": 1,
+		"get_trace.requests|result=ok":       0,
+		"get_trace.requests|result=err":      1,
+		"find_traces.requests|result=ok":     0,
+		"find_traces.requests|result=err":    1,
+		"get_services.requests|result=ok":    0,
+		"get_services.requests|result=err":   1,
 	}
 
 	existingKeys := []string{
-		"get_operations.errLatency.P50",
+		"get_operations.latency|result=err.P50",
 	}
 
 	nonExistentKeys := []string{
-		"get_operations.okLatency.P50",
+		"get_operations.latency|result=ok.P50",
 		"get_trace.responses.P50",
-		"query.okLatency.P50", // this is not exhaustive
+		"query.latency|result=ok.P50", // this is not exhaustive
 	}
 
 	checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys)