Opentelemetry-java: Exploded dependencies for API

[iNikem]

Compere the following dependencies reports:

io.opentelemetry:opentelemetry-api:0.4.1
|    |    |    \--- io.opentelemetry:opentelemetry-context-prop:0.4.1
|    |    |         \--- io.grpc:grpc-context:1.28.0

vs

io.opentelemetry:opentelemetry-api:0.5.0-SNAPSHOT
|    |    |    +--- io.grpc:grpc-bom:1.28.0
|    |    |    |    \--- io.grpc:grpc-context:1.28.0 (c)
|    |    |    +--- com.google.guava:guava-bom:28.2-android
|    |    |    |    \--- com.google.guava:guava:28.2-android (c)
|    |    |    +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |    +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |    \--- io.opentelemetry:opentelemetry-context-prop:0.5.0-SNAPSHOT
|    |    |         +--- io.grpc:grpc-bom:1.28.0 (*)
|    |    |         +--- com.google.guava:guava-bom:28.2-android (*)
|    |    |         +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |         +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |         \--- io.grpc:grpc-context -> 1.28.0

API version 0.5.0 clearly has brought many more dependencies with it. I really hope this is just a publishing bug.

[Oberon00]

We definitely must not depend on guava in the API and I see no reason why zipkin should be in there. EDIT: #1250 might be related?

[carlosalberto]

We definitely must not depend on guava in the API and I see no reason why zipkin should be in there.

Definitely. The boms are fine though.

[bogdandrutu]

@iNikem it is all about publishing (we do not depend on any other artifact compared to 0.4.0). I don't know how boms work, but it looks like we depend only on the bom and no concrete artifact.

[bogdandrutu]

It looks like guava made it to the API, I will investigate that.

[iNikem]

Good :) Then only publishing should be fixed :)

[bogdandrutu]

@iNikem I checked and guava seems to be only in tests as dependency (which is fine). Can you tell me what command you used to generate the input in the description?

[iNikem]

• Take auto-instr master,
• change opentelemetry to 0.5.0-SNAPSHOT in gradle/dependencies.gradle
• cd instrumentation/servlet (or any other instrumentation)
• ../../gradlew depe --configuration runtimeClasspath
• Look for opentelemetry-api

[bogdandrutu]

./gradlew opentelemetry-api:dependencies --configuration runtimeClasspath

runtimeClasspath - Runtime classpath of source set 'main'.
+--- project :opentelemetry-context-prop
|    +--- io.grpc:grpc-context -> 1.28.0
|    +--- io.grpc:grpc-bom:1.28.0
|    |    \--- io.grpc:grpc-context:1.28.0 (c)
|    +--- com.google.guava:guava-bom:28.2-android
|    +--- com.google.protobuf:protobuf-bom:3.11.4
|    \--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
+--- io.grpc:grpc-bom:1.28.0 (*)
+--- com.google.guava:guava-bom:28.2-android
+--- com.google.protobuf:protobuf-bom:3.11.4
\--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2

I don't see a non bom dependency for guava. In your description there is a guava

io.opentelemetry:opentelemetry-api:0.5.0-SNAPSHOT
|    |    |    +--- io.grpc:grpc-bom:1.28.0
|    |    |    |    \--- io.grpc:grpc-context:1.28.0 (c)
|    |    |    +--- com.google.guava:guava-bom:28.2-android
|    |    |    |    **\--- com.google.guava:guava:28.2-android (c)**
|    |    |    +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |    +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |    \--- io.opentelemetry:opentelemetry-context-prop:0.5.0-SNAPSHOT
|    |    |         +--- io.grpc:grpc-bom:1.28.0 (*)
|    |    |         +--- com.google.guava:guava-bom:28.2-android (*)
|    |    |         +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |         +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |         \--- io.grpc:grpc-context -> 1.28.0

[Oberon00]

Even just a BOM can cause version conflicts at build time, can't it?

[iNikem]

@bogdandrutu Tried again:

servlet % ../../gradlew depe --configuration runtimeClasspath --refresh-dependencies

> Task :instrumentation:servlet:dependencies

------------------------------------------------------------
Project :instrumentation:servlet
------------------------------------------------------------

runtimeClasspath - Runtime classpath of source set 'main'.
+--- project :auto-tooling
|    +--- project :auto-bootstrap
|    |    +--- io.opentelemetry:opentelemetry-api:0.5.0-SNAPSHOT
|    |    |    +--- io.grpc:grpc-bom:1.28.0
|    |    |    |    \--- io.grpc:grpc-context:1.28.0 (c)
|    |    |    +--- com.google.guava:guava-bom:28.2-android
|    |    |    |    \--- com.google.guava:guava:28.2-android (c)
|    |    |    +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |    +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |    \--- io.opentelemetry:opentelemetry-context-prop:0.5.0-SNAPSHOT
|    |    |         +--- io.grpc:grpc-bom:1.28.0 (*)
|    |    |         +--- com.google.guava:guava-bom:28.2-android (*)
|    |    |         +--- com.google.protobuf:protobuf-bom:3.11.4
|    |    |         +--- io.zipkin.reporter2:zipkin-reporter-bom:2.12.2
|    |    |         \--- io.grpc:grpc-context -> 1.28.0

[iNikem]

Your output is of opentelemetry-api. I check the dependencies of the client of opentelemetry-api.

[iNikem]

I think I know the reason. In top level build.gradle on line 181 you define dependencies for ALL projects:

        implementation platform(boms.grpc)
                platform(boms.guava),
                platform(boms.protobuf),
                platform(boms.zipkin_reporter)

Meaning that even api project will have guava and Zipkin as dependency and all clients of api will have them as runtime dependencies. I don't think such global dependency declaration is a good thing.

[bogdandrutu]

Meaning that even api project will have guava and Zipkin as dependency and all clients of api will have them as runtime dependencies. I don't think such global dependency declaration is a good thing.

We have the bom dependency not a concrete dependency on any of these.

[bogdandrutu]

Your output is of opentelemetry-api. I check the dependencies of the client of opentelemetry-api.

I am confused about that, what do you mean? I think guava concrete artifact (not the bom) comes from the auto-instrumentation not from us.

[iNikem]

I have made an empty Gradle project with the following build.gradle file:

plugins {
    id 'java'
}

group 'org.example'
version '1.0-SNAPSHOT'

repositories {
    mavenCentral()
    maven { url 'https://oss.jfrog.org/artifactory/oss-snapshot-local' }
}

dependencies {
//    compile('io.opentelemetry:opentelemetry-api:0.5.0-SNAPSHOT')
    compile('com.google.guava:guava:20.0')
}

task deps {
    doLast{
        configurations.runtimeClasspath.files.each { println "$it.name" }
    }
}

If I have only dependency on guava version 20, then running ./gradlew deps will give me:

> Task :deps
guava-20.0.jar

If I add dependency on opentelemetry-api:0.5.0-SNAPSHOT, then running ./gradlew deps will give me:

> Task :deps
opentelemetry-api-0.5.0-SNAPSHOT.jar
opentelemetry-context-prop-0.5.0-SNAPSHOT.jar
guava-28.2-android.jar
grpc-context-1.28.0.jar
failureaccess-1.0.1.jar
listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar
jsr305-3.0.2.jar
checker-compat-qual-2.5.5.jar
error_prone_annotations-2.3.4.jar
j2objc-annotations-1.3.jar

If I change version of opentelemetry-api to 0.4.0, then running ./gradlew deps will give me:

opentelemetry-api-0.4.0.jar
guava-20.0.jar
opentelemetry-context-prop-0.4.0.jar
grpc-context-1.28.0.jar

I stay firm on my claim that 0.5.0-SNAPSHOT has introduced unwanted dependencies.

And if I remove guava from my build and leave only api:0.5.0-SNAPSHOT, I will get:

> Task :deps
opentelemetry-api-0.5.0-SNAPSHOT.jar
opentelemetry-context-prop-0.5.0-SNAPSHOT.jar
grpc-context-1.28.0.jar

[trask]

Oh, thanks for that explanation, makes sense to me now.

And yes, I agree, adding the opentelemetry-api dependency should not inadvertently affect the version of guava that an app happens to be using.

Great catch before 0.5.0 was even released!

[iNikem]

At last I fully understand what is going on :) Bogdan is right, but that does not void the problem :)

api module publishes pom file, which has dependencyManagement section. There pom file imports guava-bom:28.2-android.

Now any project which uses otel-java api module and declares itself a guava dependency will have that dependency version overwritten by dependencyManagement from api pom file. Which may be undesirable in a lot of cases. As api itself does not depend on guava neither compile nor run time, this version overriding is not needed at all.

[bogdandrutu]

@iNikem yes that was my point, you got that.

[anuraaga]

IMO dependency management in Gradle is still an unsolved problem. Have seen so many problems with different approaches.

I thought this issue might be an issue with Gradle metadata being incorrect and wouldn't affect Maven. But needs some investigation since I notice the POM says Gradle metadata is published, see comments on beginner of this file

https://repo.maven.apache.org/maven2/io/opentelemetry/opentelemetry-api/0.4.1/opentelemetry-api-0.4.1.pom

But I can't find the Gradle .module file in the repo. Not sure what behavior to expect in this case. Is something filtering out the Gradle metadata upstream instead of just disabling it in the build?

[iNikem]

I don't see any deep problem here, actually. api publishes a constraint on an artifact that it does use/need. Solution: remove this unneeded dependency. Other modules, e.g. sdk, actually depend on some specific version of guava. Thus they declare it and require it.