Skip to content

Commit 0cd8a07

Browse files
objectiseryurishkuro
objectiser
authored and
yurishkuro
committed
Specify metric name/tags via annotation (#1096)
* Specify metric name/tags via annotation Signed-off-by: Gary Brown <[email protected]> * Update changelog Signed-off-by: Gary Brown <[email protected]> * Revert back to 'requests' and make the operation name a tag Signed-off-by: Gary Brown <[email protected]> * Add more details to the changelog Signed-off-by: Gary Brown <[email protected]>
1 parent b2aa771 commit 0cd8a07

File tree

3 files changed

+55
-40
lines changed

3 files changed

+55
-40
lines changed

Diff for: ‎CHANGELOG.md

+22-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,28 @@ Changes by Version
88

99
##### Breaking Changes
1010

11-
- Consolidate query metrics and include result tag ([#1075](https://github.com/jaegertracing/jaeger/pull/1075), [@objectiser](https://github.com/objectiser)
12-
- 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)
11+
- 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))
12+
13+
For example, sample of metrics produced for `find_traces` operation before:
14+
15+
```
16+
jaeger_find_traces_attempts 1
17+
jaeger_find_traces_errLatency_bucket{le="0.005"} 0
18+
jaeger_find_traces_errors 0
19+
jaeger_find_traces_okLatency_bucket{le="0.005"} 0
20+
jaeger_find_traces_responses_bucket{le="0.005"} 1
21+
jaeger_find_traces_successes 1
22+
```
23+
24+
And now:
25+
26+
```
27+
jaeger_query_latency_bucket{operation="find_traces",result="err",le="0.005"} 0
28+
jaeger_query_latency_bucket{operation="find_traces",result="ok",le="0.005"} 2
29+
jaeger_query_requests{operation="find_traces",result="err"} 0
30+
jaeger_query_requests{operation="find_traces",result="ok"} 2
31+
jaeger_query_responses_bucket{operation="find_traces",le="0.005"} 2
32+
```
1333

1434

1535
1.7.0 (2018-09-19)

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

+9-14
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ type ReadMetricsDecorator struct {
3434
}
3535

3636
type queryMetrics struct {
37-
Errors metrics.Counter
38-
Successes metrics.Counter
39-
Responses metrics.Timer //used as a histogram, not necessary for GetTrace
40-
ErrLatency metrics.Timer
41-
OKLatency metrics.Timer
37+
Errors metrics.Counter `metric:"requests" tags:"result=err"`
38+
Successes metrics.Counter `metric:"requests" tags:"result=ok"`
39+
Responses metrics.Timer `metric:"responses"` //used as a histogram, not necessary for GetTrace
40+
ErrLatency metrics.Timer `metric:"latency" tags:"result=err"`
41+
OKLatency metrics.Timer `metric:"latency" tags:"result=ok"`
4242
}
4343

4444
func (q *queryMetrics) emit(err error, latency time.Duration, responses int) {
@@ -63,15 +63,10 @@ func NewReadMetricsDecorator(spanReader spanstore.Reader, metricsFactory metrics
6363
}
6464
}
6565

66-
func buildQueryMetrics(namespace string, metricsFactory metrics.Factory) *queryMetrics {
67-
scoped := metricsFactory.Namespace(namespace, nil)
68-
qMetrics := &queryMetrics{
69-
Errors: scoped.Counter("", map[string]string{"result": "err"}),
70-
Successes: scoped.Counter("", map[string]string{"result": "ok"}),
71-
Responses: scoped.Timer("responses", nil),
72-
ErrLatency: scoped.Timer("latency", map[string]string{"result": "err"}),
73-
OKLatency: scoped.Timer("latency", map[string]string{"result": "ok"}),
74-
}
66+
func buildQueryMetrics(operation string, metricsFactory metrics.Factory) *queryMetrics {
67+
qMetrics := &queryMetrics{}
68+
scoped := metricsFactory.Namespace("", map[string]string{"operation": operation})
69+
metrics.Init(qMetrics, scoped, nil)
7570
return qMetrics
7671
}
7772

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

+24-24
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,23 @@ func TestSuccessfulUnderlyingCalls(t *testing.T) {
4343
mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{})
4444
counters, gauges := mf.Snapshot()
4545
expecteds := map[string]int64{
46-
"get_operations|result=ok": 1,
47-
"get_operations|result=err": 0,
48-
"get_trace|result=ok": 1,
49-
"get_trace|result=err": 0,
50-
"find_traces|result=ok": 1,
51-
"find_traces|result=err": 0,
52-
"get_services|result=ok": 1,
53-
"get_services|result=err": 0,
46+
"requests|operation=get_operations|result=ok": 1,
47+
"requests|operation=get_operations|result=err": 0,
48+
"requests|operation=get_trace|result=ok": 1,
49+
"requests|operation=get_trace|result=err": 0,
50+
"requests|operation=find_traces|result=ok": 1,
51+
"requests|operation=find_traces|result=err": 0,
52+
"requests|operation=get_services|result=ok": 1,
53+
"requests|operation=get_services|result=err": 0,
5454
}
5555

5656
existingKeys := []string{
57-
"get_operations.latency|result=ok.P50",
58-
"get_trace.responses.P50",
59-
"find_traces.latency|result=ok.P50", // this is not exhaustive
57+
"latency|operation=get_operations|result=ok.P50",
58+
"responses|operation=get_trace.P50",
59+
"latency|operation=find_traces|result=ok.P50", // this is not exhaustive
6060
}
6161
nonExistentKeys := []string{
62-
"get_operations.latency|result=err.P50",
62+
"latency|operation=get_operations|result=err.P50",
6363
}
6464

6565
checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys)
@@ -96,24 +96,24 @@ func TestFailingUnderlyingCalls(t *testing.T) {
9696
mrs.FindTraces(context.Background(), &spanstore.TraceQueryParameters{})
9797
counters, gauges := mf.Snapshot()
9898
expecteds := map[string]int64{
99-
"get_operations|result=ok": 0,
100-
"get_operations|result=err": 1,
101-
"get_trace|result=ok": 0,
102-
"get_trace|result=err": 1,
103-
"find_traces|result=ok": 0,
104-
"find_traces|result=err": 1,
105-
"get_services|result=ok": 0,
106-
"get_services|result=err": 1,
99+
"requests|operation=get_operations|result=ok": 0,
100+
"requests|operation=get_operations|result=err": 1,
101+
"requests|operation=get_trace|result=ok": 0,
102+
"requests|operation=get_trace|result=err": 1,
103+
"requests|operation=find_traces|result=ok": 0,
104+
"requests|operation=find_traces|result=err": 1,
105+
"requests|operation=get_services|result=ok": 0,
106+
"requests|operation=get_services|result=err": 1,
107107
}
108108

109109
existingKeys := []string{
110-
"get_operations.latency|result=err.P50",
110+
"latency|operation=get_operations|result=err.P50",
111111
}
112112

113113
nonExistentKeys := []string{
114-
"get_operations.latency|result=ok.P50",
115-
"get_trace.responses.P50",
116-
"query.latency|result=ok.P50", // this is not exhaustive
114+
"latency|operation=get_operations|result=ok.P50",
115+
"responses|operation=get_trace.P50",
116+
"latency|operation=query|result=ok.P50", // this is not exhaustive
117117
}
118118

119119
checkExpectedExistingAndNonExistentCounters(t, counters, expecteds, gauges, existingKeys, nonExistentKeys)

0 commit comments

Comments
 (0)