diff --git a/CHANGELOG.md b/CHANGELOG.md index c2755cf0f91..e6a6d8d7fa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,28 @@ Changes by Version ##### Breaking Changes -- Consolidate query metrics and include result tag ([#1075](https://github.com/jaegertracing/jaeger/pull/1075), [@objectiser](https://github.com/objectiser) -- Make the metrics produced by jaeger query scoped to the query component, and generated for all span readers (not just ES) ([#1074](https://github.com/jaegertracing/jaeger/pull/1074), [@objectiser](https://github.com/objectiser) +- Various changes around metrics produced by jaeger-query: Names scoped to the query component, generated for all span readers (not just ES), consolidate query metrics and include result tag ([#1074](https://github.com/jaegertracing/jaeger/pull/1074), [#1075](https://github.com/jaegertracing/jaeger/pull/1075) and [#1096](https://github.com/jaegertracing/jaeger/pull/1096), [@objectiser](https://github.com/objectiser)) + +For example, sample of metrics produced for `find_traces` operation before: + +``` +jaeger_find_traces_attempts 1 +jaeger_find_traces_errLatency_bucket{le="0.005"} 0 +jaeger_find_traces_errors 0 +jaeger_find_traces_okLatency_bucket{le="0.005"} 0 +jaeger_find_traces_responses_bucket{le="0.005"} 1 +jaeger_find_traces_successes 1 +``` + +And now: + +``` +jaeger_query_latency_bucket{operation="find_traces",result="err",le="0.005"} 0 +jaeger_query_latency_bucket{operation="find_traces",result="ok",le="0.005"} 2 +jaeger_query_requests{operation="find_traces",result="err"} 0 +jaeger_query_requests{operation="find_traces",result="ok"} 2 +jaeger_query_responses_bucket{operation="find_traces",le="0.005"} 2 +``` 1.7.0 (2018-09-19) diff --git a/storage/spanstore/metrics/decorator.go b/storage/spanstore/metrics/decorator.go index c013ddda5b5..dcf256d18bb 100644 --- a/storage/spanstore/metrics/decorator.go +++ b/storage/spanstore/metrics/decorator.go @@ -34,11 +34,11 @@ type ReadMetricsDecorator struct { } type queryMetrics struct { - Errors metrics.Counter - Successes metrics.Counter - Responses metrics.Timer //used as a histogram, not necessary for GetTrace - ErrLatency metrics.Timer - OKLatency metrics.Timer + Errors metrics.Counter `metric:"requests" tags:"result=err"` + Successes metrics.Counter `metric:"requests" tags:"result=ok"` + Responses metrics.Timer `metric:"responses"` //used as a histogram, not necessary for GetTrace + ErrLatency metrics.Timer `metric:"latency" tags:"result=err"` + OKLatency metrics.Timer `metric:"latency" tags:"result=ok"` } func (q *queryMetrics) emit(err error, latency time.Duration, responses int) { @@ -63,15 +63,10 @@ func NewReadMetricsDecorator(spanReader spanstore.Reader, metricsFactory metrics } } -func buildQueryMetrics(namespace string, metricsFactory metrics.Factory) *queryMetrics { - scoped := metricsFactory.Namespace(namespace, nil) - qMetrics := &queryMetrics{ - Errors: scoped.Counter("", map[string]string{"result": "err"}), - Successes: scoped.Counter("", 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"}), - } +func buildQueryMetrics(operation string, metricsFactory metrics.Factory) *queryMetrics { + qMetrics := &queryMetrics{} + scoped := metricsFactory.Namespace("", map[string]string{"operation": operation}) + metrics.Init(qMetrics, scoped, nil) return qMetrics } diff --git a/storage/spanstore/metrics/decorator_test.go b/storage/spanstore/metrics/decorator_test.go index 0fe9f352fe4..93121d3949e 100644 --- a/storage/spanstore/metrics/decorator_test.go +++ b/storage/spanstore/metrics/decorator_test.go @@ -43,23 +43,23 @@ func TestSuccessfulUnderlyingCalls(t *testing.T) { mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{}) counters, gauges := mf.Snapshot() expecteds := map[string]int64{ - "get_operations|result=ok": 1, - "get_operations|result=err": 0, - "get_trace|result=ok": 1, - "get_trace|result=err": 0, - "find_traces|result=ok": 1, - "find_traces|result=err": 0, - "get_services|result=ok": 1, - "get_services|result=err": 0, + "requests|operation=get_operations|result=ok": 1, + "requests|operation=get_operations|result=err": 0, + "requests|operation=get_trace|result=ok": 1, + "requests|operation=get_trace|result=err": 0, + "requests|operation=find_traces|result=ok": 1, + "requests|operation=find_traces|result=err": 0, + "requests|operation=get_services|result=ok": 1, + "requests|operation=get_services|result=err": 0, } existingKeys := []string{ - "get_operations.latency|result=ok.P50", - "get_trace.responses.P50", - "find_traces.latency|result=ok.P50", // this is not exhaustive + "latency|operation=get_operations|result=ok.P50", + "responses|operation=get_trace.P50", + "latency|operation=find_traces|result=ok.P50", // this is not exhaustive } nonExistentKeys := []string{ - "get_operations.latency|result=err.P50", + "latency|operation=get_operations|result=err.P50", } checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys) @@ -96,24 +96,24 @@ func TestFailingUnderlyingCalls(t *testing.T) { mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{}) counters, gauges := mf.Snapshot() expecteds := map[string]int64{ - "get_operations|result=ok": 0, - "get_operations|result=err": 1, - "get_trace|result=ok": 0, - "get_trace|result=err": 1, - "find_traces|result=ok": 0, - "find_traces|result=err": 1, - "get_services|result=ok": 0, - "get_services|result=err": 1, + "requests|operation=get_operations|result=ok": 0, + "requests|operation=get_operations|result=err": 1, + "requests|operation=get_trace|result=ok": 0, + "requests|operation=get_trace|result=err": 1, + "requests|operation=find_traces|result=ok": 0, + "requests|operation=find_traces|result=err": 1, + "requests|operation=get_services|result=ok": 0, + "requests|operation=get_services|result=err": 1, } existingKeys := []string{ - "get_operations.latency|result=err.P50", + "latency|operation=get_operations|result=err.P50", } nonExistentKeys := []string{ - "get_operations.latency|result=ok.P50", - "get_trace.responses.P50", - "query.latency|result=ok.P50", // this is not exhaustive + "latency|operation=get_operations|result=ok.P50", + "responses|operation=get_trace.P50", + "latency|operation=query|result=ok.P50", // this is not exhaustive } checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys)