linkerd2 icon indicating copy to clipboard operation
linkerd2 copied to clipboard

Distributed Tracing Service Name should be dynamic

Open whiskeysierra opened this issue 2 years ago • 8 comments

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 avatar Jul 24 '23 11:07 whiskeysierra

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"

whiskeysierra avatar Jul 27 '23 11:07 whiskeysierra

Hi @whiskeysierra

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

adleong avatar Jul 27 '23 17:07 adleong

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?

harsh020 avatar Aug 02 '23 18:08 harsh020

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

rye-sw avatar Aug 19 '23 07:08 rye-sw

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.

adleong avatar Aug 24 '23 00:08 adleong

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

jeremyswerdlow avatar Dec 10 '23 00:12 jeremyswerdlow

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.

alpeb avatar Dec 14 '23 10:12 alpeb

I'm currently looking into this

GrigoriyMikhalkin avatar Mar 17 '24 17:03 GrigoriyMikhalkin