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

track warning about zipkin dependency #509

Open
codefromthecrypt opened this issue Jan 5, 2024 · 6 comments
Open

track warning about zipkin dependency #509

codefromthecrypt opened this issue Jan 5, 2024 · 6 comments
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@codefromthecrypt
Copy link
Contributor

This warning says brave has a dependency on zipkin, which is currently true for a transient reason. Once that goes away, the note here should be removed:

IMPORTANT: Remember that Brave by default adds Zipkin as a dependency. If you want to use just Wavefront and you're using classpath dependant solutions such as Spring Boot, you might be required to exclude the transitive dependency on Zipkin when using Brave (e.g. via exlcuding the `io.zipkin.reporter2` group).

The reason is that brave temporarily pinned the zipkin reporter version in case the api drifted. No one removed that in 3 years, so it should be done shortly and once micrometer uses an updated version of brave, the above warning should go away: https://github.com/openzipkin/brave/blob/master/brave/pom.xml#L43-L49

Note: those who aren't using wavefront and choosing to use zipkin-reporter-brave will still have a sneaky dep on zipkin2 until a major version release of zipkin-reporter which would remove these deps. This would only affect those adding the zipkin reporter dependency so is a different thing than the wavefront thing, rather just a note fyi openzipkin/zipkin-reporter-java#182

@codefromthecrypt
Copy link
Contributor Author

This will be fixed in Brave 6, as most of the zipkin dependency was to help transition from functions deprecated 2 or more years ago.

@jonatan-ivanov jonatan-ivanov added the type: task A general task label Jan 5, 2024
@jonatan-ivanov jonatan-ivanov added this to the 1.x milestone Jan 5, 2024
@jonatan-ivanov jonatan-ivanov added documentation Improvements or additions to documentation and removed type: task A general task labels Jan 5, 2024
@codefromthecrypt
Copy link
Contributor Author

@codefromthecrypt
Copy link
Contributor Author

note: otel is also pinning the zipkin types. I updated this, but they are still pinning them anyway. This doesn't affect the brave module directly, but as integration tests use a shared classpath between otel and brave, there are some glitches until merged and released open-telemetry/opentelemetry-java#6129

@codefromthecrypt
Copy link
Contributor Author

#511 will fix most of this except the actual doc change. The zipkin libs are still pulled in by otel and I added notes about what to do when open-telemetry/opentelemetry-java#6129 is released

@codefromthecrypt
Copy link
Contributor Author

specifically otel is locking into a version of zipkin-reporter-brave which still has a dep on zipkin types. It is easy to fix as I've raised the PR needed by OTEL and released library versions of what's needed to complete this.

The rest I have to leave with you all as I have to get to my day job which has nothing to do with tracing.

@codefromthecrypt
Copy link
Contributor Author

finally I did a brain dump here for those who care to parse it.

TL;DR; zipkin server needs to be able to upgrade to a different floor version of java than instrumentation at some point, zipkin-reporter3 was needed not just for this issue, but also our own reasons

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants