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

OSGi package import missing in context-slf4j #1248

Closed
helgoboss opened this issue Aug 27, 2020 · 4 comments · Fixed by #1252
Closed

OSGi package import missing in context-slf4j #1248

helgoboss opened this issue Aug 27, 2020 · 4 comments · Fixed by #1252
Labels

Comments

@helgoboss
Copy link

When deploying brave-context-slf4j-5.12.5.jar into an OSGi runtime (in our case Equinox/Liferay), the packages resolve successfully. However, when actually using it (MDCScopeDecorator), I get this exception:

Caused by: java.lang.ClassNotFoundException: brave.internal.CorrelationContext cannot be found by io.zipkin.brave.context-slf4j_5.12.5

I re-osgified the bundle via bnd-maven-plugin which generated the following Import-Package instruction:

Import-Package: brave;version="[5.12,6)",brave.baggage;version="[5.12,6)",brave.internal;version="[5.12,6)";braveinternal=true,brave.propagation;version="[5.12,6)",org.slf4j

This fixes the issue.

@helgoboss helgoboss added the bug label Aug 27, 2020
@codefromthecrypt
Copy link
Member

eek we are glad you are helping test! this will affect the other ones too. I suppose when shading we forgot to also update the osgi metadata, so we need a comment on that part to ensure folks don't forget next time.

codefromthecrypt pushed a commit that referenced this issue Sep 11, 2020
It is my understanding that unless we shade something, we have to add
internal import for the few utilities we use.

Fixes #1248
@codefromthecrypt
Copy link
Member

I spiked this, but not sure the intended result is achieved.. check? #1252

@helgoboss
Copy link
Author

I built brave-context-slf4j-5.12.6-SNAPSHOT.jar in your PR and the result works for me. The only remaining Import-Package difference to my re-osgified bundle is that yours doesn't import brave;version="[5.12,6)". bnd-maven-plugin somehow detected this import but I don't know why and if it's really needed. In my use case at least it wasn't needed.

@codefromthecrypt
Copy link
Member

@helgoboss thanks for such a close look. I also audited that, and I suppose this is limiting to primary imports. It is an interesting thing, but context does not require all packages. Usually you construct the "context" thing prior to building the tracer, so that might help make sense of why it might not need "brave." import, even if that's why you are building it.

codefromthecrypt pushed a commit that referenced this issue Sep 11, 2020
It is my understanding that unless we shade something, we have to add
internal import for the few utilities we use.

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

Successfully merging a pull request may close this issue.

2 participants