Skip to content

Commit 108c29b

Browse files
authored
Restore "operation" name in the metrics response (jaegertracing#5673)
## Which problem is this PR solving? - Resolves jaegertracing#5672 - Due to oversight, a bug was introduced in jaegertracing#5539 which changed the attribute name expected by the UI from `operation` to `span_name` ## Description of the changes - Revert those changes ## How was this change tested? `(cd docker-compose/monitor/ && make build dev)` <img width="1910" alt="image" src="https://github.com/jaegertracing/jaeger/assets/3523016/79400073-75f1-4db5-8bef-9a4c439d2852"> ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Yuri Shkuro <[email protected]>
1 parent cd1b4ea commit 108c29b

File tree

4 files changed

+22
-24
lines changed

4 files changed

+22
-24
lines changed

plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ type Translator struct {
3131
// New returns a new Translator.
3232
func New(spanNameLabel string) Translator {
3333
return Translator{
34-
// "span_name" is the label name that Jaeger UI expects.
35-
labelMap: map[string]string{spanNameLabel: "span_name"},
34+
// "operation" is the label name that Jaeger UI expects.
35+
labelMap: map[string]string{spanNameLabel: "operation"},
3636
}
3737
}
3838

plugin/metrics/prometheus/metricsstore/dbmodel/to_domain_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestToDomainMetricsFamily(t *testing.T) {
4848

4949
wantMetricLabels := map[string]string{
5050
"label_key": "label_value",
51-
"span_name": "span_name_value", // assert the name is translated to a Jaeger-friendly label.
51+
"operation": "span_name_value", // assert the name is translated to a Jaeger-friendly label.
5252
}
5353
assert.Len(t, mf.Metrics, 1)
5454
for _, ml := range mf.Metrics[0].Labels {

plugin/metrics/prometheus/metricsstore/reader.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type (
5555
metricsTranslator dbmodel.Translator
5656
latencyMetricName string
5757
callsMetricName string
58-
operationLabel string
58+
operationLabel string // name of the attribute that contains span name / operation
5959
}
6060

6161
promQueryParams struct {
@@ -90,7 +90,7 @@ func NewMetricsReader(cfg config.Configuration, logger *zap.Logger, tracer trace
9090
return nil, fmt.Errorf("failed to initialize prometheus client: %w", err)
9191
}
9292

93-
operationLabel := "span_name"
93+
const operationLabel = "span_name"
9494

9595
mr := &MetricsReader{
9696
client: promapi.NewAPI(client),

plugin/metrics/prometheus/metricsstore/reader_test.go

+17-19
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package metricsstore
1616

1717
import (
1818
"context"
19-
"fmt"
2019
"io"
2120
"net"
2221
"net/http"
@@ -183,7 +182,7 @@ func TestGetLatencies(t *testing.T) {
183182
wantName: "service_operation_latencies",
184183
wantDescription: "0.95th quantile latency, grouped by service & operation",
185184
wantLabels: map[string]string{
186-
"span_name": "/OrderResult",
185+
"operation": "/OrderResult",
187186
"service_name": "emailservice",
188187
},
189188
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_bucket{service_name =~ "emailservice", ` +
@@ -215,7 +214,7 @@ func TestGetLatencies(t *testing.T) {
215214
wantName: "service_operation_latencies",
216215
wantDescription: "0.95th quantile latency, grouped by service & operation",
217216
wantLabels: map[string]string{
218-
"span_name": "/OrderResult",
217+
"operation": "/OrderResult",
219218
"service_name": "emailservice",
220219
},
221220
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(span_metrics_duration_bucket{service_name =~ "emailservice", ` +
@@ -234,7 +233,7 @@ func TestGetLatencies(t *testing.T) {
234233
wantName: "service_operation_latencies",
235234
wantDescription: "0.95th quantile latency, grouped by service & operation",
236235
wantLabels: map[string]string{
237-
"span_name": "/OrderResult",
236+
"operation": "/OrderResult",
238237
"service_name": "emailservice",
239238
},
240239
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_seconds_bucket{service_name =~ "emailservice", ` +
@@ -257,7 +256,7 @@ func TestGetLatencies(t *testing.T) {
257256

258257
m, err := reader.GetLatencies(context.Background(), &params)
259258
require.NoError(t, err)
260-
assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription)
259+
assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription)
261260
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
262261
})
263262
}
@@ -286,7 +285,7 @@ func TestGetCallRates(t *testing.T) {
286285
wantName: "service_operation_call_rate",
287286
wantDescription: "calls/sec, grouped by service & operation",
288287
wantLabels: map[string]string{
289-
"span_name": "/OrderResult",
288+
"operation": "/OrderResult",
290289
"service_name": "emailservice",
291290
},
292291
wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", ` +
@@ -317,7 +316,7 @@ func TestGetCallRates(t *testing.T) {
317316
wantName: "service_operation_call_rate",
318317
wantDescription: "calls/sec, grouped by service & operation",
319318
wantLabels: map[string]string{
320-
"span_name": "/OrderResult",
319+
"operation": "/OrderResult",
321320
"service_name": "emailservice",
322321
},
323322
wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", ` +
@@ -335,7 +334,7 @@ func TestGetCallRates(t *testing.T) {
335334
wantName: "service_operation_call_rate",
336335
wantDescription: "calls/sec, grouped by service & operation",
337336
wantLabels: map[string]string{
338-
"span_name": "/OrderResult",
337+
"operation": "/OrderResult",
339338
"service_name": "emailservice",
340339
},
341340
wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", ` +
@@ -357,7 +356,7 @@ func TestGetCallRates(t *testing.T) {
357356

358357
m, err := reader.GetCallRates(context.Background(), &params)
359358
require.NoError(t, err)
360-
assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription)
359+
assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription)
361360
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
362361
})
363362
}
@@ -387,7 +386,7 @@ func TestGetErrorRates(t *testing.T) {
387386
wantName: "service_operation_error_rate",
388387
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
389388
wantLabels: map[string]string{
390-
"span_name": "/OrderResult",
389+
"operation": "/OrderResult",
391390
"service_name": "emailservice",
392391
},
393392
wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
@@ -439,7 +438,7 @@ func TestGetErrorRates(t *testing.T) {
439438
wantName: "service_operation_error_rate",
440439
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
441440
wantLabels: map[string]string{
442-
"span_name": "/OrderResult",
441+
"operation": "/OrderResult",
443442
"service_name": "emailservice",
444443
},
445444
wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
@@ -458,7 +457,7 @@ func TestGetErrorRates(t *testing.T) {
458457
wantName: "service_operation_error_rate",
459458
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
460459
wantLabels: map[string]string{
461-
"span_name": "/OrderResult",
460+
"operation": "/OrderResult",
462461
"service_name": "emailservice",
463462
},
464463
wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
@@ -481,7 +480,7 @@ func TestGetErrorRates(t *testing.T) {
481480

482481
m, err := reader.GetErrorRates(context.Background(), &params)
483482
require.NoError(t, err)
484-
assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription)
483+
assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription)
485484
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
486485
})
487486
}
@@ -945,17 +944,16 @@ func prepareMetricsReaderAndServer(t *testing.T, config config.Configuration, wa
945944
return reader, mockPrometheus
946945
}
947946

948-
func assertMetrics(t *testing.T, testName string, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) {
947+
func assertMetrics(t *testing.T, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) {
949948
assert.Len(t, gotMetrics.Metrics, 1)
950949
assert.Equal(t, wantName, gotMetrics.Name)
951950
assert.Equal(t, wantDescription, gotMetrics.Help)
952951
mps := gotMetrics.Metrics[0].MetricPoints
953952
assert.Len(t, mps, 1)
954953

955954
// logging for expected and actual labels
956-
fmt.Printf("Test Name: %s\n", testName)
957-
fmt.Printf("Expected labels: %v\n", wantLabels)
958-
fmt.Printf("Actual labels: %v\n", gotMetrics.Metrics[0].Labels)
955+
t.Logf("Expected labels: %v\n", wantLabels)
956+
t.Logf("Actual labels: %v\n", gotMetrics.Metrics[0].Labels)
959957

960958
// There is no guaranteed order of labels, so we need to take the approach of using a map of expected values.
961959
labels := gotMetrics.Metrics[0].Labels
@@ -968,8 +966,8 @@ func assertMetrics(t *testing.T, testName string, gotMetrics *metrics.MetricFami
968966
assert.Empty(t, wantLabels)
969967

970968
// Additional logging to show that all expected labels were found and matched
971-
fmt.Printf("Remaining expected labels after matching: %v\n", wantLabels)
972-
fmt.Printf("\n")
969+
t.Logf("Remaining expected labels after matching: %v\n", wantLabels)
970+
t.Logf("\n")
973971

974972
assert.Equal(t, int64(1620351786), mps[0].Timestamp.GetSeconds())
975973

0 commit comments

Comments
 (0)