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

Implement OTel HTTP semantic conventions #929

Closed
mmanciop opened this issue Jan 2, 2025 · 8 comments
Closed

Implement OTel HTTP semantic conventions #929

mmanciop opened this issue Jan 2, 2025 · 8 comments

Comments

@mmanciop
Copy link

mmanciop commented Jan 2, 2025

The attributes of the HTTP spans generated by Micrometer Tracing do not conform with OpenTelemetry semantic conventions [1], reducing the usefulness of the telemetry generated, as the end user needs different queries to filter semconv compliant telemetry from other than is not compliant.

From the PoV of backwards compatibility, generating span metadata that is semconv compliant should be opt-in until the next major release, where the behavior should be switched to opt-out for compatibility for current users and their queries.

[1] https://opentelemetry.io/docs/specs/semconv/http/

@shakuzen
Copy link
Member

shakuzen commented Jan 6, 2025

Thanks for opening the issue. It's on our roadmap to explore if we can make it easier to opt-in to using an ObservationConvention that adopts a specific version of the corresponding OTel semantic conventions. As you mention, we can't just update instrumentation everywhere because that would be a breaking change. In the meantime, it's always possible for users to provide their own ObservationConvention implementation that uses a version of the OTel semantic conventions they want. For example, see this section of the Spring Framework documentation on customizing the convention for e.g. Spring MVC instrumentation. Note, all of these things are outside of this repo because this repo does not deal with conventions - that's up to instrumentation and if instrumenting via Micrometer Observation API, the ObservationConvention configured.

@jonatan-ivanov
Copy link
Member

I would like to emphasize what Tommy said above: assuming you use Spring, Micrometer does not deal with naming spans and span tags/annotations, the instrumentation inside Spring Framework does.

Also, in addition to looking into supporting the OTel naming conventions: currently, the OTel naming conventions are not stable yet (this means that they can break you in prod). Instrumentation in Spring supports user-provided conventions so you can replace what you want to but we recommend using the defaults.

@mmanciop
Copy link
Author

mmanciop commented Jan 7, 2025

Hi @jonatan-ivanov, the OTel semantic conventions, as a whole, may not be stable. (There will always be some new semconv being worked on.)

Some namespaces are stable, however, like the all-important http.* one (declared stable more than a year ago [1]) that lead me to open this issue.

[1] https://opentelemetry.io/blog/2023/http-conventions-declared-stable/

@shakuzen
Copy link
Member

shakuzen commented Jan 7, 2025

I've opened micrometer-metrics/micrometer#5794 to track the general idea. As mentioned in previous comments, there is nothing in this repository dealing with HTTP conventions for OTel. The conventions are up to the specific instrumentation (e.g. Spring MVC, Apache HTTP client, etc). So I don't expect any change to happen in this repository toward achieving the outcome you are requesting. It would either need to be done via an ObservationConvention you implement yourself for the instrumentations you are using, or one maintained by someone else. We will explore in micrometer-metrics/micrometer#5794 what we can/should do to make that easier. For that reason, we'll close this issue in lieu of micrometer-metrics/micrometer#5794.

@shakuzen shakuzen closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2025
@jonatan-ivanov
Copy link
Member

Some namespaces are stable, however, like the all-important http.* one (declared stable more than a year ago [1]) that lead me to open this issue.

Please check the link above: the OTel naming conventions are not stable yet, it says the following:

NOTE: Although this is for stable semantic conventions, the artifact still has the -alpha and comes with no compatibility guarantees. The goal is to mark this artifact stable.

I understand that the specs are declared stable for HTTP but the published artifacts are still not stable yet and that's what projects consume. As far as I know, the Boot/Framework teams consider those artifacts unstable.

@mmanciop
Copy link
Author

mmanciop commented Jan 7, 2025

Some namespaces are stable, however, like the all-important http.* one (declared stable more than a year ago [1]) that lead me to open this issue.

Please check the link above: the OTel naming conventions are not stable yet, it says the following:

NOTE: Although this is for stable semantic conventions, the artifact still has the -alpha and comes with no compatibility guarantees. The goal is to mark this artifact stable.

I understand that the specs are declared stable for HTTP but the published artifacts are still not stable yet and that's what projects consume. As far as I know, the Boot/Framework teams consider those artifacts unstable.

I understand that the artifact might not be stable, but as a profane of the Spring Boot ecosystem, it sounds to me like an implementation detail.

From an OpenTelemetry perspective, the semconvs for HTTP are stable, so one could implement them without the artifact, and have reasonable expectations of them remaining correct going forward.

This being said, if you folks feel like that artifact is a key interface for you with the OpenTelemetry ecosystem, I understand why you may want to wait for it to stabilize.

@mmanciop
Copy link
Author

mmanciop commented Jan 7, 2025

However, if the concern is that, as a whole, OTel semconvs are not yet stable, that realistically is going to be the case for the as long as OpenTelemetry will be a thriving community.

I am not convinced that it would be a good choice to withhold the implementation of the parts that are indeed stable and widely adopted like HTTP, and soon others like DB.

@jonatan-ivanov
Copy link
Member

I understand that the artifact might not be stable, but as a profane of the Spring Boot ecosystem, it sounds to me like an implementation detail.

Breaking changes in these artifacts are real (we learned this the hard way), they are marked as -alpha so that they can be broken in the future but you can ask the Spring Boot/Framework teams if they have changed their minds.

From an OpenTelemetry perspective, the semconvs for HTTP are stable, so one could implement them without the artifact, and have reasonable expectations of them remaining correct going forward.

I guess projects are looking to OTel to provide these artifacts instead of picking up the maintenance burden themselves. This was discussed multiple times on OTel meetings, OTel maintainers are aware (I can bring this up again). I know at least one project that instead of depending on the unstable artifacts, they hardcoded the stable key names but it was only a couple (2-3?) keys. I guess no one wants to do this for bigger things but again, this would be a Spring Boot/Framework question. As we mentioned above, you can always inject your own ObservationConventions that can use the OTel semconv; Spring Boot/Framework let you use whatever convention you want to.

This being said, if you folks feel like that artifact is a key interface for you with the OpenTelemetry ecosystem, I understand why you may want to wait for it to stabilize.

I would say for instrumentation with OTel semconv support, it is but this is not a Micrometer question but the question of the library/framework that instruments itself.

However, if the concern is that, as a whole, OTel semconvs are not yet stable, that realistically is going to be the case for the as long as OpenTelemetry will be a thriving community.

I am not convinced that it would be a good choice to withhold the implementation of the parts that are indeed stable and widely adopted like HTTP, and soon others like DB.

As far as I know, they stable/unstable parts (by spec) already separated. Please take a look at the link above:

  • io.opentelemetry.semconv:opentelemetry-semconv is for stable (by spec) semconv
  • io.opentelemetry.semconv:opentelemetry-semconv-incubating is for unstable (by spec) semconv

As OTel stabilizes things in the spec, they move classes from incubating to "stable". I think your concern above should not be a problem: opentelemetry-semconv should be a stable artifact in the future that contains the stable parts of the semconv spec (while a separate artifact contains the unstable parts).

Please feel free to @mention us if you open an issue for any projects regarding this, we might be able to help.

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

No branches or pull requests

3 participants