Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the default metrics namespace/prefix for span RED metrics in Jaeger query #1072

Merged
merged 6 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .chloggen/fix_red-metrics.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the fix also fixing monolithic?

Copy link
Collaborator Author

@frzifus frzifus Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it looks like that was never supported? I can not find anything about the STORAGE_METRICS_TYPE.
21054a4#diff-2dd4203d1e98f65856b756889e3b555cd1909bf0133911ad1cc7bf0667f8573bR295

╰─❯ grep -r "METRICS_STORAGE_TYPE"     
grep: bin/manager: Übereinstimmungen in Binärdatei
internal/manifests/queryfrontend/query_frontend.go:				Name:  "METRICS_STORAGE_TYPE",
internal/manifests/queryfrontend/query_frontend_test.go:			env:  []corev1.EnvVar{{Name: "METRICS_STORAGE_TYPE", Value: "prometheus"}, {Name: "PROMETHEUS_SERVER_URL", Value: "http://prometheus:9091"}},
internal/manifests/queryfrontend/query_frontend_test.go:				{Name: "METRICS_STORAGE_TYPE", Value: "prometheus"},

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, monolithic doesn't support red metrics at the moment


# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Use new 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: |
frzifus marked this conversation as resolved.
Show resolved Hide resolved
Use the new RED metrics default namespace `traces.span.metrics` for retrieval from Prometheus.
Since OpenTelemetry Collector version 0.109.0 the default namespace is set to traces.span.metrics.
The namespace taken into account by jaeger-query can be configured via a TempoStack CR entry.
To achieve this the Operator will set the jaeger-query `--prometheus.query.namespace=` flag.
Since Jaeger version 1.62, jaeger-query uses `traces.span.metrics` as default too.

Example how to overwrite the default namespace with the old default before `0.109.0` by configuring it in the CR:
frzifus marked this conversation as resolved.
Show resolved Hide resolved
```
apiVersion: tempo.grafana.com/v1alpha1
kind: TempoStack
...
spec:
template:
queryFrontend:
jaegerQuery:
enabled: true
monitorTab:
enabled: true
prometheusEndpoint: "http://myPromInstance:9090"
redMetricsNamespace: "custom"
```
More details can be found here:
- https://github.com/jaegertracing/jaeger/pull/6007
- https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/34485
7 changes: 7 additions & 0 deletions apis/tempo/v1alpha1/tempostack_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,13 @@ type JaegerQueryMonitor struct {
// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Prometheus endpoint"
PrometheusEndpoint string `json:"prometheusEndpoint"`
// REDMetricsNamespace defines the a prefix used retrieve span rate, error, and duration (RED) metrics.
// By default it is set to `traces.span.metrics` following the default namespace of the OpenTelemetry Collector since Version 0.109.0.
// +optional
// +kubebuilder:validation:Optional
// +kubebuilder:default:=traces.span.metrics
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="RED Metric Namespace"
REDMetricsNamespace string `json:"redMetricsNamespace"`
}

// IngressSpec defines Jaeger Query Ingress options.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,12 @@ spec:
on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
displayName: Prometheus endpoint
path: template.queryFrontend.jaegerQuery.monitorTab.prometheusEndpoint
- description: REDMetricsNamespace defines the a prefix used retrieve span rate,
error, and duration (RED) metrics. By default it is set to `traces.span.metrics`
following the default namespace of the OpenTelemetry Collector since Version
0.109.0.
displayName: RED Metric Namespace
path: template.queryFrontend.jaegerQuery.monitorTab.redMetricsNamespace
- description: Resources defines resources for this component, this will override
the calculated resources derived from total
displayName: Resources
Expand Down
6 changes: 6 additions & 0 deletions bundle/community/manifests/tempo.grafana.com_tempostacks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2470,6 +2470,12 @@ spec:
PrometheusEndpoint defines the endpoint to the Prometheus instance that contains the span rate, error, and duration (RED) metrics.
For instance on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
type: string
redMetricsNamespace:
default: traces.span.metrics
description: |-
REDMetricsNamespace defines the a prefix used retrieve span rate, error, and duration (RED) metrics.
By default it is set to `traces.span.metrics` following the default namespace of the OpenTelemetry Collector since Version 0.109.0.
type: string
type: object
resources:
description: Resources defines resources for this component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,12 @@ spec:
on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
displayName: Prometheus endpoint
path: template.queryFrontend.jaegerQuery.monitorTab.prometheusEndpoint
- description: REDMetricsNamespace defines the a prefix used retrieve span rate,
error, and duration (RED) metrics. By default it is set to `traces.span.metrics`
following the default namespace of the OpenTelemetry Collector since Version
0.109.0.
displayName: RED Metric Namespace
path: template.queryFrontend.jaegerQuery.monitorTab.redMetricsNamespace
- description: Resources defines resources for this component, this will override
the calculated resources derived from total
displayName: Resources
Expand Down
6 changes: 6 additions & 0 deletions bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2470,6 +2470,12 @@ spec:
PrometheusEndpoint defines the endpoint to the Prometheus instance that contains the span rate, error, and duration (RED) metrics.
For instance on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
type: string
redMetricsNamespace:
default: traces.span.metrics
description: |-
REDMetricsNamespace defines the a prefix used retrieve span rate, error, and duration (RED) metrics.
By default it is set to `traces.span.metrics` following the default namespace of the OpenTelemetry Collector since Version 0.109.0.
type: string
type: object
resources:
description: Resources defines resources for this component,
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/tempo.grafana.com_tempostacks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2466,6 +2466,12 @@ spec:
PrometheusEndpoint defines the endpoint to the Prometheus instance that contains the span rate, error, and duration (RED) metrics.
For instance on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
type: string
redMetricsNamespace:
default: traces.span.metrics
description: |-
REDMetricsNamespace defines the a prefix used retrieve span rate, error, and duration (RED) metrics.
By default it is set to `traces.span.metrics` following the default namespace of the OpenTelemetry Collector since Version 0.109.0.
type: string
type: object
resources:
description: Resources defines resources for this component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,12 @@ spec:
on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
displayName: Prometheus endpoint
path: template.queryFrontend.jaegerQuery.monitorTab.prometheusEndpoint
- description: REDMetricsNamespace defines the a prefix used retrieve span rate,
error, and duration (RED) metrics. By default it is set to `traces.span.metrics`
following the default namespace of the OpenTelemetry Collector since Version
0.109.0.
displayName: RED Metric Namespace
path: template.queryFrontend.jaegerQuery.monitorTab.redMetricsNamespace
- description: Resources defines resources for this component, this will override
the calculated resources derived from total
displayName: Resources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,12 @@ spec:
on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
displayName: Prometheus endpoint
path: template.queryFrontend.jaegerQuery.monitorTab.prometheusEndpoint
- description: REDMetricsNamespace defines the a prefix used retrieve span rate,
error, and duration (RED) metrics. By default it is set to `traces.span.metrics`
following the default namespace of the OpenTelemetry Collector since Version
0.109.0.
displayName: RED Metric Namespace
path: template.queryFrontend.jaegerQuery.monitorTab.redMetricsNamespace
- description: Resources defines resources for this component, this will override
the calculated resources derived from total
displayName: Resources
Expand Down
1 change: 1 addition & 0 deletions docs/spec/tempo.grafana.com_tempostacks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ spec: # TempoStackSpec defines the desired st
monitorTab: # MonitorTab defines the monitor tab configuration.
enabled: false # Enabled enables the monitor tab in the Jaeger console. The PrometheusEndpoint must be configured to enable this feature.
prometheusEndpoint: "" # PrometheusEndpoint defines the endpoint to the Prometheus instance that contains the span rate, error, and duration (RED) metrics. For instance on OpenShift this is set to https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
redMetricsNamespace: "traces.span.metrics" # REDMetricsNamespace defines the a prefix used retrieve span rate, error, and duration (RED) metrics. By default it is set to `traces.span.metrics` following the default namespace of the OpenTelemetry Collector since Version 0.109.0.
servicesQueryDuration: "" # ServicesQueryDuration defines how long the services will be available in the services list
tempoQuery: # TempoQuery defines options specific to the Tempoo Query component.
resources: # Resources defines resources for this component, this will override the calculated resources derived from total
Expand Down
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.
fmt.Sprintf("--prometheus.query.namespace=%s", tempo.Spec.Template.QueryFrontend.JaegerQuery.MonitorTab.REDMetricsNamespace),
},
}
// If the endpoint matches Prometheus on OpenShift, configure TLS and token based auth
Expand Down
8 changes: 5 additions & 3 deletions internal/manifests/queryfrontend/query_frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,15 +621,16 @@ func TestBuildQueryFrontendWithJaegerMonitorTab(t *testing.T) {
JaegerQuery: v1alpha1.JaegerQuerySpec{
Enabled: true,
MonitorTab: v1alpha1.JaegerQueryMonitor{
Enabled: true,
PrometheusEndpoint: "http://prometheus:9091",
Enabled: true,
PrometheusEndpoint: "http://prometheus:9091",
REDMetricsNamespace: "test",
},
},
},
},
},
},
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=test"},
env: []corev1.EnvVar{{Name: "METRICS_STORAGE_TYPE", Value: "prometheus"}, {Name: "PROMETHEUS_SERVER_URL", Value: "http://prometheus:9091"}},
},
{
Expand Down Expand Up @@ -657,6 +658,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
1 change: 1 addition & 0 deletions tests/e2e-openshift/red-metrics/03-install-tempo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ spec:
monitorTab:
enabled: true
prometheusEndpoint: https://thanos-querier.openshift-monitoring.svc.cluster.local:9091
redMetricsNamespace: ""
ingress:
type: route