Skip to content

Fix instrumentation failure when constructor has @WithSpan annotation #13929

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

laurit
Copy link
Contributor

@laurit laurit commented May 27, 2025

Resolves #13920
This PR excludes constructors from @WithSpan instrumentation since it currently does not work at all. This is not ideal as @WithSpan annotation is allowed on constructors, I don't think we can change that.
Implementing @WithSpan support for constructors is slightly problematic because as far as I can tell on constructors byte buddy does not allow OnMethodExit with onThrowable.

@laurit laurit requested a review from a team as a code owner May 27, 2025 11:56
Copy link
Contributor

@JonasKunz JonasKunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related bytebuddy issue:
raphw/byte-buddy#375

If we would still want to support tracing constructors with the limitation of exceptions not being detected we could simply create a separate WithSpanConstructorAdvice which omits the onThrowable.

@trask
Copy link
Member

trask commented May 27, 2025

Related bytebuddy issue: raphw/byte-buddy#375

If we would still want to support tracing constructors with the limitation of exceptions not being detected we could simply create a separate WithSpanConstructorAdvice which omits the onThrowable.

I think I'd prefer the consistency of never capture constructor calls vs only capture successful constructor calls

@WithSpan annotation is allowed on constructors, I don't think we can change that

if we change that in 3.0, I'm guessing it would only be a "compile-time" breaking change (and wouldn't break any already compiled classes using the annotation)?

@laurit
Copy link
Contributor Author

laurit commented May 28, 2025

if we change that in 3.0, I'm guessing it would only be a "compile-time" breaking change (and wouldn't break any already compiled classes using the annotation)?

I believe so, but have not attempted to verify this.

@laurit laurit merged commit c36eedd into open-telemetry:main May 28, 2025
88 checks passed
@laurit laurit deleted the with-span-constructor branch May 28, 2025 05:59
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.

@WithSpan on constructor causes all @WithSpan on that class to be ignored
4 participants