Skip to content

[tempo-distributed] drop jaeger-ui related ports and config completely #3963

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.

Sign up for GitHub

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

Open
zoomoid wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[tempo-distributed] drop jaeger-ui related ports and config completely #3963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1d3d0c
refactor(tempo-distributed): include servicemonitor for (jaeger) temp…
zoomoid Oct 29, 2025
0e8b73c
refactor: tempo-query templates don't work anything like they used to
zoomoid Oct 29, 2025
eb23f9a
refactor: change port of query-frontend service and ingress
zoomoid Oct 29, 2025
9a08b9f
chore: bump chart version
zoomoid Oct 30, 2025
8fbdfb7
chore: address review comment on service port
zoomoid Dec 3, 2025
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
2 changes: 1 addition & 1 deletion charts/tempo-distributed/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: tempo-distributed
description: Grafana Tempo in MicroService mode
type: application
version: 1.52.3
version: 1.53.0
appVersion: 2.9.0
engine: gotpl
home: https://grafana.com/docs/tempo/latest/
Expand Down
8 changes: 3 additions & 5 deletions charts/tempo-distributed/templates/query-frontend/deployment-query-frontend.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ spec:
{{- end }}
{{- if .Values.queryFrontend.query.enabled }}
- args:
- -config=/conf/tempo.yaml
- -config=/conf/tempo-query.yaml
{{- with .Values.queryFrontend.query.extraArgs }}
{{- toYaml . | nindent 12 }}
{{- end }}
Expand All @@ -129,10 +129,8 @@ spec:
imagePullPolicy: {{ .Values.tempo.image.pullPolicy }}
name: tempo-query
ports:
- containerPort: {{ .Values.queryFrontend.service.port }}
name: jaeger-ui
- containerPort: 16687
name: jaeger-metrics
- containerPort: 7777
name: tempo-query
{{- with .Values.queryFrontend.query.extraEnv }}
env:
{{- toYaml . | nindent 12 }}
Expand Down
2 changes: 1 addition & 1 deletion charts/tempo-distributed/templates/query-frontend/ingress-query-frontend.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if and .Values.queryFrontend.query.enabled .Values.queryFrontend.ingress.enabled -}}
{{- if .Values.queryFrontend.ingress.enabled -}}
Copy link
Collaborator

@QuentinBisson QuentinBisson Oct 31, 2025

Choose a reason for hiding this comment

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

Is there any reason to change this?

Copy link
Contributor Author

@zoomoid zoomoid Dec 3, 2025

Choose a reason for hiding this comment

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

the tempo-query service evolved to be gRPC-only, and I see very little use in having an HTTP ingress for a gRPC service the way it currently is set up.

However, similar to the remark below, I might be work considering if the ingress needs to be kept in its entirety, I think the original idea went along the same lines of setting the queryFrontend.service.port to the HTTP metrics port, because that was the last remaining (somewhat sensible) HTTP endpoint to use for this entire service.

The extra predicate in the conditional can of course remain, but since the ingress is fully customizable with respect to host, path and so forth, technically you don't have to use it for the tempo-query service alone and could also use it to expose the 3200 metrics endpoint of query-frontend (whatever might be the reason to do this).

Also, if the ingress used to just be for tempo-query, why not put its configuration in the values under queryFrontend.query?

{{ $dict := dict "ctx" . "component" "query-frontend" }}
{{- $ingressApiIsStable := eq (include "tempo.ingress.isStable" .) "true" -}}
{{- $ingressSupportsIngressClassName := eq (include "tempo.ingress.supportsIngressClassName" .) "true" -}}
Expand Down
11 changes: 5 additions & 6 deletions charts/tempo-distributed/templates/query-frontend/service-query-frontend-discovery.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,11 @@ spec:
appProtocol: {{ .Values.queryFrontend.appProtocol.grpc }}
{{- end }}
{{- if .Values.queryFrontend.query.enabled }}
- name: tempo-query-jaeger-ui
port: {{ .Values.queryFrontend.service.port }}
targetPort: {{ .Values.queryFrontend.service.port }}
- name: tempo-query-metrics
port: 16687
targetPort: jaeger-metrics
- name: tempo-query
port: 7777
targetPort: tempo-query
protocol: TCP
appProtocol: grpc
{{- end }}
publishNotReadyAddresses: true
selector:
Expand Down
9 changes: 4 additions & 5 deletions charts/tempo-distributed/templates/query-frontend/service-query-frontend.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ spec:
appProtocol: {{ .Values.queryFrontend.appProtocol.grpc }}
{{- end }}
{{- if .Values.queryFrontend.query.enabled }}
- name: tempo-query-jaeger-ui
- name: tempo-query
port: {{ .Values.queryFrontend.service.port }}
targetPort: {{ .Values.queryFrontend.service.port }}
- name: tempo-query-metrics
port: 16687
targetPort: jaeger-metrics
targetPort: tempo-query
protocol: TCP
appProtocol: grpc
{{- end }}
{{- if .Values.queryFrontend.service.loadBalancerIP }}
loadBalancerIP: {{ .Values.queryFrontend.service.loadBalancerIP }}
Expand Down
1 change: 0 additions & 1 deletion charts/tempo-distributed/templates/query-frontend/servicemonitor-jaeger-query.yaml
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions charts/tempo-distributed/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,8 +1005,8 @@ queryFrontend:
# -- Docker image tag for the query-frontend image. Overrides `tempo.image.tag`
tag: null
service:
# -- Port of the query-frontend service
port: 16686
# -- Port of the tempo-query component in the queryFrontend service
port: 7777
# -- Annotations for queryFrontend service
annotations: {}
# -- Labels for queryFrontend service
Expand Down