diff --git a/CHANGELOG.md b/CHANGELOG.md index a9d35217891..c2755cf0f91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Changes by Version ================== +1.8.0 (unreleased) +------------------ + +#### Backend Changes + +##### 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) + + 1.7.0 (2018-09-19) ------------------ diff --git a/storage/spanstore/metrics/decorator.go b/storage/spanstore/metrics/decorator.go index e277253c206..c013ddda5b5 100644 --- a/storage/spanstore/metrics/decorator.go +++ b/storage/spanstore/metrics/decorator.go @@ -34,16 +34,14 @@ 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 + 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 +64,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("", 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"}), + } return qMetrics } diff --git a/storage/spanstore/metrics/decorator_test.go b/storage/spanstore/metrics/decorator_test.go index 9be83f7a696..0fe9f352fe4 100644 --- a/storage/spanstore/metrics/decorator_test.go +++ b/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|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, } 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|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, } 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)