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

opentelemetry-sdk has toxic dependencies #576

Closed
codefromthecrypt opened this issue Sep 29, 2019 · 2 comments
Closed

opentelemetry-sdk has toxic dependencies #576

codefromthecrypt opened this issue Sep 29, 2019 · 2 comments

Comments

@codefromthecrypt
Copy link
Contributor

opentelemetry-sdk is aimed to be the highest point of re-use, fulfilling the uniform implementation promise formerly promised by census, while opentelemetry-api aims to forward the promises made by OpenTracing. However, it includes some dependencies which will certainly create conflicts in applications and pin versions. While #575 is the most important to watch, opentelemetry-sdk is an empty promise of uniform instrumentation when it includes toxic dependencies.

If it isn't obvious, guava and protobuf are toxic deps, and gson seems accidental. Guava conflicts are so routine they don't need explanation. Protobuf has made api changes in the past, which means it can also pin applications. Moreover, libraries often increased their own dependency bloat silently over time, which result in silent sprawl. The simplest way is to push dependencies like this to codec layers which are themselves optional.

I know there have been some claims of things like "free timestamp conversion" in other pull requests, but that's a really weak reason to pin dependencies in the lower most code of an application. Please fix this

$ ./gradlew :opentelemetry-sdk:dependencies
--snip--

+--- com.google.auto.value:auto-value-annotations:1.6.2
+--- com.google.errorprone:error_prone_annotations:2.3.2
+--- com.google.code.findbugs:jsr305:3.0.2
+--- project :opentelemetry-api
|    +--- com.google.auto.value:auto-value-annotations:1.6.2
|    +--- com.google.errorprone:error_prone_annotations:2.3.2
|    +--- com.google.code.findbugs:jsr305:3.0.2
|    \--- io.grpc:grpc-context:1.20.0
+--- com.google.protobuf:protobuf-java:3.7.1
\--- com.google.protobuf:protobuf-java-util:3.7.1
     +--- com.google.protobuf:protobuf-java:3.7.1
     +--- com.google.guava:guava:20.0
     +--- com.google.errorprone:error_prone_annotations:2.3.2
     \--- com.google.code.gson:gson:2.7

--snip--

+--- com.google.auto.value:auto-value-annotations:1.6.2
+--- com.google.errorprone:error_prone_annotations:2.3.2
+--- com.google.code.findbugs:jsr305:3.0.2
+--- project :opentelemetry-api
|    +--- com.google.auto.value:auto-value-annotations:1.6.2
|    +--- com.google.errorprone:error_prone_annotations:2.3.2
|    +--- com.google.code.findbugs:jsr305:3.0.2
|    \--- io.grpc:grpc-context:1.20.0
+--- com.google.protobuf:protobuf-java:3.7.1
\--- com.google.protobuf:protobuf-java-util:3.7.1
     +--- com.google.protobuf:protobuf-java:3.7.1
     +--- com.google.guava:guava:20.0
     +--- com.google.errorprone:error_prone_annotations:2.3.2
     \--- com.google.code.gson:gson:2.7
@bogdandrutu
Copy link
Member

So I think this issue will be closed because the only left item will be grpc-context which you filed a different issue for that.

@codefromthecrypt
Copy link
Contributor Author

Thanks for looking into this Bogdan. It is true that inside applications that can move their dependencies forward, that going guava 20+ will help avoid known compat problems. This is an instrumentation library, though. You can't make such assumptions. For example, this supports Java 7 runtime. It is asymmetric to suggest all apps are going to have compatible versions of guava.

If you desire to keep guava as a strict dep, you should point loudly in the runtime compatibility part of the README that instrumented applications must use guava 20+.

Eventually, thought, you will get into the same boat as everyone else, which is that you either don't depend strictly on guava or you shade it. I'm alerting you now to help you make the right change when it isn't expensive and doesn't cause support problems. For example, newbies very very often struggle with classpath. This hops on support channels, and sometimes those support channels you personally are not on. In other words, it causes problems for other people. The best way out is to not interfere with the classpath at all in low-level libraries.

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

No branches or pull requests

2 participants