client_java icon indicating copy to clipboard operation
client_java copied to clipboard

Add exemplars support for Micrometer Tracing

Open jonatan-ivanov opened this issue 3 years ago • 4 comments
trafficstars

This PR adds exemplars support for Micrometer Tracing. Micrometer Tracing is the successor of Spring Cloud Sleuth in terms of providing an abstraction layer over tracing libraries (i.e.: Brave and OTel) and it provides distributed tracing support for the Spring portfolio (Spring Boot 3.x, Spring Framework 6.x, etc).

There is one thing missing from this PR where I would like to ask for some guidance from the maintainers: there is no static configuration. Since Micrometer Tracing needs a Tracer instance and it supports both Brave + OTel (Brave does not have static config) this might be problematic since users can use their own Tracer instance that might be different than the Prometheus client is aware of.

@fstab @tomwilkie

jonatan-ivanov avatar Aug 10 '22 01:08 jonatan-ivanov

Also, do you happen to know why the javadoc goal is failing? The error message does not seem to be very helpful.

jonatan-ivanov avatar Aug 10 '22 04:08 jonatan-ivanov

Hello again, I had another look.

  • Regarding javadoc: The directory with the source code is literally called micrometer.tracing and not micrometer/tracing.
  • Regarding static initialization: Is there any chance to create a class that would be detected by Spring or CDI so that we can get the Micrometer tracer injected? I guess if there's neither a static variable that we can use nor a way to hook into Spring or CDI initialization then users will need to configure the Examplar sampler programmatically.

fstab avatar Sep 09 '22 17:09 fstab

Heads-up: I renamed master to main.

fstab avatar Sep 11 '22 18:09 fstab

javadoc: 🤦🏼 🤣 I'm sorry.

static initialization I'm not 100% sure I understand this:

Is there any chance to create a class that would be detected by Spring or CDI so that we can get the Micrometer tracer injected?

Spring and other CDI solutions will initialize their context after class loading (by creating non-static instances) so even if they inject something to a static context, that will happen (or can easily happen without control) after the java client's static initializers were executed. Also, since they can (and will) inject the registry, the global/static components are less useful in those environments (in fact they can cause trouble and make testing hard/impossible).

This is my honest feedback, I don't want to offend anyone or step on any toes (also, I'm biased): I think the Prometheus Java Client should focus on letting CDI/users configure things in non-static ways (providing builders with defaults and factory methods are completely ok). I think it could also use a ServiceLoader for static access. I assume that this ship has sailed (and might not be the scope of this PR) so I guess the options are:

  1. Keep this as-is saying that users will not get static initializer support for Micrometer Tracing they should do this manually or their framework should do this for them. (Right now, this is enough for me from Micrometer/Micrometer Tracing/Spring side.)

  2. Only support OTel: this means handling two cases: 1. only OTel, 2. Micrometer Tracing (with OTel bridge) + OTel 3. Neither.

  3. Support everything (this is basically reimplementing Spring Boot auto-configuration or other auto-configuration in other frameworks), the cases are:

    1. OTel only
    2. Micrometer Tracing (with OTel bridge) + OTel
    3. Micrometer Tracing (with Brave bridge) + Brave
    4. None of the above
    5. Lots of invalid cases like Micrometer Tracing with OTel bridge + Brave/something else/nothing (or Micrometer Tracing with OTel bridge and Brave bridge together)

If Micrometer Tracing adds support for another Tracing library or Prometheus Client adds support for another Tracing library (i.e.: Brave) or both, this will increase quickly

jonatan-ivanov avatar Oct 12 '22 02:10 jonatan-ivanov