Skip to content

Commit

Permalink
Restore "operation" name in the metrics response (#5673)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Resolves #5672
- Due to oversight, a bug was introduced in #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]>
  • Loading branch information
yurishkuro committed Jun 22, 2024
1 parent c80b8e1 commit d35c985
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 24 deletions.
4 changes: 2 additions & 2 deletions plugin/metrics/prometheus/metricsstore/dbmodel/to_domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ type Translator struct {
// New returns a new Translator.
func New(spanNameLabel string) Translator {
return Translator{
// "span_name" is the label name that Jaeger UI expects.
labelMap: map[string]string{spanNameLabel: "span_name"},
// "operation" is the label name that Jaeger UI expects.
labelMap: map[string]string{spanNameLabel: "operation"},
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestToDomainMetricsFamily(t *testing.T) {

wantMetricLabels := map[string]string{
"label_key": "label_value",
"span_name": "span_name_value", // assert the name is translated to a Jaeger-friendly label.
"operation": "span_name_value", // assert the name is translated to a Jaeger-friendly label.
}
assert.Len(t, mf.Metrics, 1)
for _, ml := range mf.Metrics[0].Labels {
Expand Down
4 changes: 2 additions & 2 deletions plugin/metrics/prometheus/metricsstore/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type (
metricsTranslator dbmodel.Translator
latencyMetricName string
callsMetricName string
operationLabel string
operationLabel string // name of the attribute that contains span name / operation
}

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

operationLabel := "span_name"
const operationLabel = "span_name"

mr := &MetricsReader{
client: promapi.NewAPI(client),
Expand Down
36 changes: 17 additions & 19 deletions plugin/metrics/prometheus/metricsstore/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package metricsstore

import (
"context"
"fmt"
"io"
"net"
"net/http"
Expand Down Expand Up @@ -183,7 +182,7 @@ func TestGetLatencies(t *testing.T) {
wantName: "service_operation_latencies",
wantDescription: "0.95th quantile latency, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_bucket{service_name =~ "emailservice", ` +
Expand Down Expand Up @@ -215,7 +214,7 @@ func TestGetLatencies(t *testing.T) {
wantName: "service_operation_latencies",
wantDescription: "0.95th quantile latency, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(span_metrics_duration_bucket{service_name =~ "emailservice", ` +
Expand All @@ -234,7 +233,7 @@ func TestGetLatencies(t *testing.T) {
wantName: "service_operation_latencies",
wantDescription: "0.95th quantile latency, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `histogram_quantile(0.95, sum(rate(duration_seconds_bucket{service_name =~ "emailservice", ` +
Expand All @@ -257,7 +256,7 @@ func TestGetLatencies(t *testing.T) {

m, err := reader.GetLatencies(context.Background(), &params)
require.NoError(t, err)
assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription)
assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription)
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
})
}
Expand Down Expand Up @@ -286,7 +285,7 @@ func TestGetCallRates(t *testing.T) {
wantName: "service_operation_call_rate",
wantDescription: "calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", ` +
Expand Down Expand Up @@ -317,7 +316,7 @@ func TestGetCallRates(t *testing.T) {
wantName: "service_operation_call_rate",
wantDescription: "calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", ` +
Expand All @@ -335,7 +334,7 @@ func TestGetCallRates(t *testing.T) {
wantName: "service_operation_call_rate",
wantDescription: "calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", ` +
Expand All @@ -357,7 +356,7 @@ func TestGetCallRates(t *testing.T) {

m, err := reader.GetCallRates(context.Background(), &params)
require.NoError(t, err)
assertMetrics(t, tc.name, m, tc.wantLabels, tc.wantName, tc.wantDescription)
assertMetrics(t, m, tc.wantLabels, tc.wantName, tc.wantDescription)
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
})
}
Expand Down Expand Up @@ -387,7 +386,7 @@ func TestGetErrorRates(t *testing.T) {
wantName: "service_operation_error_rate",
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
Expand Down Expand Up @@ -439,7 +438,7 @@ func TestGetErrorRates(t *testing.T) {
wantName: "service_operation_error_rate",
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(span_metrics_calls{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
Expand All @@ -458,7 +457,7 @@ func TestGetErrorRates(t *testing.T) {
wantName: "service_operation_error_rate",
wantDescription: "error rate, computed as a fraction of errors/sec over calls/sec, grouped by service & operation",
wantLabels: map[string]string{
"span_name": "/OrderResult",
"operation": "/OrderResult",
"service_name": "emailservice",
},
wantPromQlQuery: `sum(rate(calls_total{service_name =~ "emailservice", status_code = "STATUS_CODE_ERROR", ` +
Expand All @@ -481,7 +480,7 @@ func TestGetErrorRates(t *testing.T) {

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

func assertMetrics(t *testing.T, testName string, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) {
func assertMetrics(t *testing.T, gotMetrics *metrics.MetricFamily, wantLabels map[string]string, wantName, wantDescription string) {
assert.Len(t, gotMetrics.Metrics, 1)
assert.Equal(t, wantName, gotMetrics.Name)
assert.Equal(t, wantDescription, gotMetrics.Help)
mps := gotMetrics.Metrics[0].MetricPoints
assert.Len(t, mps, 1)

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

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

// Additional logging to show that all expected labels were found and matched
fmt.Printf("Remaining expected labels after matching: %v\n", wantLabels)
fmt.Printf("\n")
t.Logf("Remaining expected labels after matching: %v\n", wantLabels)
t.Logf("\n")

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

Expand Down

0 comments on commit d35c985

Please sign in to comment.