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

Distributed Tracing Service Name should be dynamic #11157

Closed
whiskeysierra opened this issue Jul 24, 2023 · 8 comments · Fixed by #13130
Closed

Distributed Tracing Service Name should be dynamic #11157

whiskeysierra opened this issue Jul 24, 2023 · 8 comments · Fixed by #13130

Comments

@whiskeysierra
Copy link

What problem are you trying to solve?

As of today, the service name that the proxy uses to emit traces is hard coded to "linkerd-proxy":

https://github.com/linkerd/linkerd2-proxy/blob/32245601bb32fa36b2f8d5997e3d806ee388eb41/linkerd/app/src/oc_collector.rs#L40

Because different services will share this name for their proxies, it makes meaningful analysis or visualisation of a whole system unnecessarily hard. A service map will typically include one big node in the center which is the "linkerd-proxy" but it's made up of data that comes from a lot of different proxies, i.e. it represents something that doesn't exist. Also any kind of trace analysis often ends up at a "linkerd-proxy" without telling you which service it belongs to.

How should the problem be solved?

Instead of being a static string, it should be controlled by an env var, ideally a template string so that e.g. a service called "my-service" can pick a template "%s-linkerd-proxy" and the service name that is used by the proxy will then be "my-service-linkerd-proxy".

Any alternatives you've considered?

I tried to look into the opentelemetry collector to rewrite those service names, but without any kind of identifying span attributes (and I couldn't find any besides the service name itself), one can't rewrite those.

How would users interact with this feature?

Env var for the linkerd-proxy sidecar or even nicer would probably be an annotation (which could then be translated into an env var by the injector).

Would you like to work on this feature?

maybe

@whiskeysierra
Copy link
Author

I did find a workaround that might be suited for most people.
If you're running the opentelemetry collector (which most probably are, because linkerd still uses opencensus), you can configure a custom processor that changes the service.name attribute.
The linkerd-proxy adds all pod labels as span attributes which means you can use those to construct a new service name:

transform/linkerd-proxy:
  error_mode: ignore
  trace_statements:
    - context: resource
      statements:
        - |
          set(
            attributes["service.name"],
            # TODO pick any labels you have/need here:
            Concat(
              [
                attributes["app.kubernetes.io/part-of"],
                attributes["app.kubernetes.io/name"]
              ],
              "-"
            )
          ) where attributes["service.name"] == "linkerd-proxy"

@adleong
Copy link
Member

adleong commented Jul 27, 2023

Hi @whiskeysierra

This definitely sounds reasonable to me. We would be happy to accept a PR along these lines.

@harsh020
Copy link
Contributor

harsh020 commented Aug 2, 2023

Hey @adleong if no one is working on this one can I?
Before that a few questions:

  1. we can achieve this by introducing an env variableSERVICE_NAME_PREFIX (as suggested by @whiskeysierra ). We can simply have the suffix linkerd_proxy, appended to the env variable (if any) to create a service name.
  2. The Opentelementry way that @whiskeysierra suggests, can we use that or it is just a way around for people using Opentelementry?

@rye-sw
Copy link

rye-sw commented Aug 19, 2023

I'm looking for this feature too! I'd like to contribute if no one is working on it.

@adleong
Copy link
Member

adleong commented Aug 24, 2023

Thanks for your interest, everyone! I'm not aware of anyone actively working on this already. And I think @whiskeysierra's suggestions above are good ones.

@jeremyswerdlow
Copy link

Is this up for grabs? Seems it's been a few months since the last comment, and not seeing any commits to address it

@alpeb
Copy link
Member

alpeb commented Dec 14, 2023

Is this up for grabs? Seems it's been a few months since the last comment, and not seeing any commits to address it

Yep, this hasn't been addressed yet.

@GrigoriyMikhalkin
Copy link
Contributor

I'm currently looking into this

sfleen added a commit to sfleen/linkerd2-proxy that referenced this issue Sep 26, 2024
Currently, we hard-code the trace service name to "linkerd-proxy". This allows this name to be configured through an env that can be set by the injector, annotations, etc.

linkerd/linkerd2#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Sep 27, 2024
Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2-proxy that referenced this issue Oct 1, 2024
Currently, we hard-code the trace service name to "linkerd-proxy". This allows this name to be configured through an env that can be set by the injector, annotations, etc.

linkerd/linkerd2#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 1, 2024
Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to linkerd/linkerd2-proxy that referenced this issue Oct 1, 2024
Currently, we hard-code the trace service name to "linkerd-proxy". This allows this name to be configured through an env that can be set by the injector, annotations, etc.

linkerd/linkerd2#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 2, 2024
Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 2, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 9, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 10, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 10, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
sfleen added a commit to sfleen/linkerd2 that referenced this issue Oct 11, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to linkerd#12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes linkerd#11157

Signed-off-by: Scott Fleener <[email protected]>
alpeb pushed a commit that referenced this issue Oct 11, 2024
Companion to linkerd/linkerd2-proxy#3245, exposes the configuration for the tracing service name in the proxy to the general linkerd config.

Similar in concept to #12371, except the configuration lives entirely within the tracing injector config instead of going through the control plane.

Fixes #11157

Signed-off-by: Scott Fleener <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants