Skip to content

Micrometer Tracing with OTEL Bridge does not honour Semantic Conventions #34132

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

Closed
jimbogithub opened this issue Feb 8, 2023 · 10 comments
Closed

Comments

@jimbogithub
Copy link

jimbogithub commented Feb 8, 2023

OpenTelemetry defines semantic conventions, e.g. https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/

micrometer-tracing-bridge-otel appears to contain code to support at least some of these but it is not activated by the spring-boot auto config. I think at one point it may have (afdb651) but was removed (13a2ea9). Hence we don't get http.client_ip, http.user_agent etc.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 8, 2023
@jimbogithub
Copy link
Author

Looks like io.micrometer.observation.ObservationConvention<T> is probably the right way to do this now? I now have some of what I need by extending org.springframework.http.server.observation.DefaultServerRequestObservationConvention and adding KeyValues. This mechanism is a nice improvement as it avoids Filter ordering issues that I had with a legacy approach.

So perhaps a micrometer-bridge-otel-semconv module is needed?

@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Feb 12, 2023

Let me give you some background/more details:

  1. OTel Semantic Conventions are not stable yet: "status: experimental" on the page you linked and also the latest version in Maven Central is still -alpha.
  2. That's one of the reason the stable/GA version of Spring Framework (or Boot) does not bring you in these since they are not production ready yet.
  3. The other reason is backward-compatibility: the outputs of the default instrumentation (that is now in Spring Framework) is compatible with the previous one (that was in Boot 2.x). Also, using OTLP, the OTel APIs or the OTel SDK does not mean you must use semantic conventions.

If you want to use the OTel semantic conventions, defining your own ObservationConvention is the way. In fact, this is one of the reasons we designed ObservationConvention: with the use of it you can define whatever convention you want to use. Right now it is in our (our as in Micrometer folks, we need buy-in from the Boot and Framework teams) future plans to provide OTel conventions as well next to the default (/backward compatible) one and you can choose which one you want. Right now doing such a thing would be very risky since semantic conventions are not stable yet.

@wilkinsona
Copy link
Member

Thanks, @jonatan-ivanov.

I'm going to close this issue for now. If and when the conventions become stable we can consider how to offer a path to migrate to them. It's too soon to do that now and we prefer not to keep an issue open if it's going to be blocked indefinitely.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2023
@wilkinsona wilkinsona removed the status: waiting-for-triage An issue we've not yet triaged label Feb 12, 2023
@tony-clarke-amdocs
Copy link

@wilkinsona @jonatan-ivanov Would you accepts PRs for this, that was opt in for enabling this OTEL semantic convention?

@scottfrederick
Copy link
Contributor

@tony-clarke-amdocs These statements are still true:

  • OTel Semantic Conventions are not stable yet: "status: experimental" on the page you linked and also the latest version in Maven Central is still -alpha.
  • That's one of the reason the stable/GA version of Spring Framework (or Boot) does not bring you in these since they are not production ready yet.

We would not accept a PR that depends on something that does not have a GA release, so Andy's comment still applies.

@fahrradflucht
Copy link

Given that some semantic conventions are now stable (e.g. HTTP spans), does it make sense to reconsider this issue? Or is your stance that the maven central package needs to be GA? Because that will obviously take way longer given that there are new conventions added for additional areas over time, but they are packaged together.

@scottfrederick
Copy link
Contributor

is your stance that the maven central package needs to be GA?

Yes, that is the case. We will not include a pre-GA version of any dependency in a GA version of Spring Boot.

@antechrestos
Copy link

@scottfrederick Would it be acceptable to provide a PR that provides a convention / documentation that can be injected through configuration to HttpWebHandlerAdapter without putting the alpha dependency as @tony-clarke-amdocs suggested ?

@bclozel
Copy link
Member

bclozel commented Jul 19, 2024

@antechrestos we would rather keep such conventions in a dedicated project that syncs up with the semantic metadata. See https://github.com/micrometer-metrics/micrometer-otel-conventions-experimental

@antechrestos
Copy link

Well... This repo does not seem quite active, last commit one year ago.
Since I have issue integrating with some APM (method instead of http.method or rather http.request.method...) I will either implement on my side or a processor step in a otel collector sidecar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants