-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(tracing): Add proxy tracing configuration to control plane helm chart #13994
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
Conversation
IIUC, this should be followed up as well by parametrizing the deployment of the injector in linkerd-jaeger, disabling it by default? |
62c78bc
to
812c616
Compare
87a4795
to
d28fd75
Compare
d28fd75
to
aa54bd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There still are issues with the whitespace suppression; when enabling tracing I see:
- emptyDir:
medium: Memory
name: linkerd-identity-end-entity- name: linkerd-podinfo
downwardAPI:
items:
- path: labels
fieldRef:
fieldPath: metadata.labels
aa54bd9
to
4364e08
Compare
should be fixed, there was some extra whitespace trimming that the templates were doing |
a7390fc
to
238b25a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting tracing.enable: true
while leaving endpoint
empty, produces the following error:
$ k -n linkerd logs -f linkerd-identity-6b66cd9c8b-c4bt7 linkerd-proxy
time="2025-06-05T15:11:38Z" level=info msg="Found pre-existing key: /var/run/linkerd/identity/end-entity/key.p8"
time="2025-06-05T15:11:38Z" level=info msg="Found pre-existing CSR: /var/run/linkerd/identity/end-entity/csr.der"
[ 0.001322s] INFO ThreadId(01) linkerd2_proxy: release 2.300.0 (67dc85a) by linkerd on 2025-06-02T22:14:32Z
[ 0.002305s] ERROR ThreadId(01) linkerd_app::env::types: Not a valid address:
[ 0.002311s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_TRACE_COLLECTOR_SVC_ADDR="" is not valid: AddrError(MissingPort)
Is this expected? What do you think about adding some validation at the template level, like what we have in _validate.tpl
?
238b25a
to
b89b429
Compare
That's super handy, I hadn't done much with helm template validation so that'll help prevent the biggest configuration footgun here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sfleen , LGTM! 👍
…chart Previously, this would require the `linkerd-jaeger` extension to be installed. This comes with its own set of issues, namely managing yet another component of linkerd. Since the tracing config is basically just environment variables and one volume mount, hoisting them up into the main control plane helm chart for the proxy injector to handle is likely the simplest and most maintainable path going forward. The injector from the `linkerd-jaeger` extension will still work for the time being, and is interoprable to some degree as long as the injector in the extension is disabled. A follow-up to this PR should add documentation around this and how users can migrate from the extension to these configs. Signed-off-by: Scott Fleener <scott@buoyant.io>
b89b429
to
f5eca66
Compare
Previously, this would require the
linkerd-jaeger
extension to be installed. This comes with its own set of issues, namely managing yet another component of linkerd.Since the tracing config is basically just environment variables and one volume mount, hoisting them up into the main control plane helm chart for the proxy injector to handle is likely the simplest and most maintainable path going forward.
The injector from the
linkerd-jaeger
extension will still work for the time being, and is interoprable to some degree as long as the injector in the extension is disabled. A follow-up to this PR should add documentation around this and how users can migrate from the extension to these configs.