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

@WithSpan annotation not functioning with 0.7.0 #913

Closed
jkwatson opened this issue Aug 6, 2020 · 13 comments · Fixed by #929
Closed

@WithSpan annotation not functioning with 0.7.0 #913

jkwatson opened this issue Aug 6, 2020 · 13 comments · Fixed by #929
Assignees
Labels
bug Something isn't working

Comments

@jkwatson
Copy link
Contributor

jkwatson commented Aug 6, 2020

A little bird told me that the shading might need to be updated for the new packaging of the extensions code.

@jkwatson jkwatson added the bug Something isn't working label Aug 6, 2020
@trask trask self-assigned this Aug 6, 2020
@iNikem
Copy link
Contributor

iNikem commented Aug 7, 2020

I will be very interested to know, why our test does not show this 🤔

@anuraaga
Copy link
Contributor

anuraaga commented Aug 7, 2020

Word on the street is it's because our unit tests don't have the runtime otel shading. Or something like that :) Smoke tests do and we don't have the annotation there.

A random idea I had recently for unrelated reasons is we could probably migrate our tests to use an actual TestAgent instead of runtime bytebuddy injection. This would be similar to how I moved the runtime bootstrap injection into Gradle - doing similar for agent would let tests match production even more closely.

@trask
Copy link
Member

trask commented Aug 8, 2020

we could probably migrate our tests to use an actual TestAgent instead of runtime bytebuddy injection

a potential downside of this is the debugging experience may require a bit more effort if launching a remote JVM

@trask
Copy link
Member

trask commented Aug 9, 2020

we could probably migrate our tests to use an actual TestAgent instead of runtime bytebuddy injection

a potential downside of this is the debugging experience may require a bit more effort if launching a remote JVM

oh, i think i understand what you were suggesting now. adding -javaagent to the gradle test JVM itself does sound very interesting.

@iNikem
Copy link
Contributor

iNikem commented Aug 9, 2020

Our tests will have to change significantly in that case. There will be much more spans captured by them when every test will have all our instrumentations applied. But it still may be a good idea.

@trask
Copy link
Member

trask commented Aug 9, 2020

I think(?) the idea is (or could be?) that we wouldn't use the full opentelemetry-javaagent, but some small delegating javaagent jar that would honor the module's existing classpath somehow.

For example, Glowroot runs tests in both "local" mode (similar to bytebuddy attach, but using disposable ClassLoader) which is nice for debugging, and "javaagent" mode which works similar to this idea, but launches separate JVM from inside the test which makes debugging less friendly (and why my comment above).

This idea sounds like it could be best of both.

@iNikem
Copy link
Contributor

iNikem commented Aug 9, 2020

Do you mean that instead of current per-module bootstrap jar we will package per-module javaagent, which has only those instrumentation that are on the classpath of this module? It may be very interesting idea indeed! :)

@anuraaga
Copy link
Contributor

@iNikem Yup that's exactly what I'm thinking :)

@trask
Copy link
Member

trask commented Aug 10, 2020

So we don't lose this idea once this issue closes out, created #936

@rjean-gilles
Copy link

Would anybody be able to tell me if the @WithSpan works for earlier versions, in particular 0.6.0? Thanks.

@trask
Copy link
Member

trask commented Aug 12, 2020

I think it was broken in 0.6.0 also. Should work in 0.5.0, or you can try latest SNAPSHOT https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/master/CONTRIBUTING.md#snapshot-builds

@trask
Copy link
Member

trask commented Aug 12, 2020

hm, latest SNAPSHOT can be tricky, because it may only interoperate with OpenTelemetry API SNAPSHOT

@rjean-gilles
Copy link

Thanks for looking it up.
I guess I'll keep creating the spans using my own helpers then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants