search

openzipkin / brave

Java distributed tracing implementation compatible with Zipkin backend services.
Apache License 2.0
2.34k stars 714 forks source link

OSGi package import missing in context-slf4j #1248

Closed helgoboss closed 3 years ago

helgoboss commented 3 years ago

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.

codefromthecrypt commented 3 years ago

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 commented 3 years ago

I spiked this, but not sure the intended result is achieved.. check? https://github.com/openzipkin/brave/pull/1252

helgoboss commented 3 years ago

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 commented 3 years ago

@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.