Skip to content

Commit

Permalink
queryfrontend: remove default RED metric namespace
Browse files Browse the repository at this point in the history
Signed-off-by: Benedikt Bongartz <[email protected]>
  • Loading branch information
frzifus committed Oct 28, 2024
1 parent 8c618d6 commit 66fde82
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
22 changes: 22 additions & 0 deletions .chloggen/fix_red-metrics.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. tempostack, tempomonolithic, github action)
component: tempostack

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Remove the default metrics namespace/prefix for span RED metrics in Jaeger query.


# One or more tracking issues related to the change
issues: [1072]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Fix RED metric retrieval from prometheus by removing the in Jaeger 1.62 introducted default RED metric namespace.
The Operator overwrites the new default namespace jaeger-query takes into account using the `--prometheus.query.namespace=` flag.
More details can be found here:
- https://github.com/jaegertracing/jaeger/pull/6007
- https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/34485
5 changes: 5 additions & 0 deletions internal/manifests/queryfrontend/query_frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,11 @@ func enableMonitoringTab(tempo v1alpha1.TempoStack, jaegerQueryContainer corev1.
// However, we do not intend to support them.
// --prometheus.query.normalize-calls
// --prometheus.query.normalize-duration
//
// NOTE: Jaeger 1.62 default namespace changed to "traces_span_metrics".
// We fallback to no namespace.
// See https://github.com/jaegertracing/jaeger/pull/6007.
"--prometheus.query.namespace=",
},
}
// If the endpoint matches Prometheus on OpenShift, configure TLS and token based auth
Expand Down
3 changes: 2 additions & 1 deletion internal/manifests/queryfrontend/query_frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ func TestBuildQueryFrontendWithJaegerMonitorTab(t *testing.T) {
},
},
},
args: []string{"--query.base-path=/", "--span-storage.type=grpc", "--grpc-storage.server=localhost:7777", "--query.bearer-token-propagation=true"},
args: []string{"--query.base-path=/", "--span-storage.type=grpc", "--grpc-storage.server=localhost:7777", "--query.bearer-token-propagation=true", "--prometheus.query.namespace="},
env: []corev1.EnvVar{{Name: "METRICS_STORAGE_TYPE", Value: "prometheus"}, {Name: "PROMETHEUS_SERVER_URL", Value: "http://prometheus:9091"}},
},
{
Expand Down Expand Up @@ -657,6 +657,7 @@ func TestBuildQueryFrontendWithJaegerMonitorTab(t *testing.T) {
"--span-storage.type=grpc",
"--grpc-storage.server=localhost:7777",
"--query.bearer-token-propagation=true",
"--prometheus.query.namespace=",
"--prometheus.tls.enabled=true",
"--prometheus.token-file=/var/run/secrets/kubernetes.io/serviceaccount/token",
"--prometheus.token-override-from-context=false",
Expand Down

0 comments on commit 66fde82

Please sign in to comment.