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

Support Spring Web MVC in library instrumentation #7552

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

mateuszrzeszutek
Copy link
Member

Part of #7312

This is pretty much a copy of the spring-webvmc-5.3:library module with s/javax/jakarta/ applied. I'm planning on removing the 5.3 instrumentation after #7312 is done.

@mateuszrzeszutek mateuszrzeszutek requested a review from a team January 11, 2023 13:13
@github-actions github-actions bot requested a review from theletterf January 11, 2023 13:13
@trask trask merged commit ca310b4 into open-telemetry:main Jan 12, 2023
@mateuszrzeszutek mateuszrzeszutek deleted the spring-webmvc-6-library branch January 12, 2023 09:26
@marcingrzejszczak
Copy link
Contributor

Why not just redirect work to the inbuilt micrometer observation mechanism (https://docs.spring.io/spring-framework/docs/current/reference/html/integration.html#integration.observability)? That way you don't have to maintain the integration at all 🤷 ?

@mateuszrzeszutek
Copy link
Member Author

The Spring conventions are very different from OTel, we'd have to implement and inject our own. KeyValues only support string values, so to implement OTel conventions correctly we'd have to do add some magic that converts e.g. ports to integers.

I agree with you that the OTel javaagent needs to support Micrometer tracing at some point in time, but right now it's much easier for us to slightly modify the existing instrumentations to work with Spring 6/Spring Boot 3. Implementing a tracing bridge might also take quite a while -- I mean, it took me over a month to implement a MeterRegistry bridge and it's still not exactly perfect (which is honestly the fault of the OTel metrics API, namely the impossibility of defining histogram buckets in the API).

@marcingrzejszczak
Copy link
Contributor

The Spring conventions are very different from OTel, we'd have to implement and inject our own. KeyValues only support string values, so to implement OTel conventions correctly we'd have to do add some magic that converts e.g. ports to integers.

I understand.

I agree with you that the OTel javaagent needs to support Micrometer tracing at some point in time, but right now it's much easier for us to slightly modify the existing instrumentations to work with Spring 6/Spring Boot 3.

Makes sense!

Implementing a tracing bridge might also take quite a while -- I mean, it took me over a month to implement a MeterRegistry bridge and it's still not exactly perfect (which is honestly the fault of the OTel metrics API, namely the impossibility of defining histogram buckets in the API).

The tracing bridge is already there https://github.com/micrometer-metrics/tracing/tree/main/micrometer-tracing-bridges/micrometer-tracing-bridge-otel

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

Successfully merging this pull request may close these issues.

3 participants