v1.47.0 Introduces a missing link on dependencies

[nihalmirpuri]

Describe the bug We are now getting this failure in our (sbt) build during a routine missingLinkDependency check:

[error] Category: Class being called not found
[error]   In artifact: opentelemetry-sdk-trace-1.47.0.jar
[error]     In class: io.opentelemetry.sdk.trace.ExtendedSdkSpanBuilder
[error]       In method:  setParentFrom(io.opentelemetry.context.propagation.ContextPropagators, java.util.Map):107
[error]       Call to: io.opentelemetry.api.incubator.propagation.ExtendedContextPropagators.extractTextMapPropagationContext(java.util.Map, io.opentelemetry.context.propagation.ContextPropagators)
[error]       Problem: Class not found: io.opentelemetry.api.incubator.propagation.ExtendedContextPropagators
[error]       --------
[error]       In method:  lambda$startAndRun$0(io.opentelemetry.api.incubator.trace.SpanRunnable):143
[error]       Call to: io.opentelemetry.api.incubator.trace.SpanRunnable.runInSpan()
[error]       Problem: Class not found: io.opentelemetry.api.incubator.trace.SpanRunnable
[error]       --------
[error]       In method:  startAndCall(io.opentelemetry.api.incubator.trace.SpanCallable, java.util.function.BiConsumer):123
[error]       Call to: io.opentelemetry.api.incubator.trace.SpanCallable.callInSpan()
[error]       Problem: Class not found: io.opentelemetry.api.incubator.trace.SpanCallable
[error]       --------

From a quick investigation, looks like https://github.com/open-telemetry/opentelemetry-java/pull/6944 removes the incubator dependency from the classpath, but I see it's still being referenced in places like ExtendedSdkSpanBuilder

Steps to reproduce

• Use v1.47.0 and review the classpath

What version and what artifacts are you using? Artifacts: opentelemetry-trace Version: 1.47.0

[jkwatson]

It's been changed to a compile-time dependency only, so if you want to use the incubator module, you'll need to add it as an explicit dependency. Not a bug. Very much intentional.

[nihalmirpuri]

Thanks for your response!

so if you want to use the incubator module

We don't want to use the incubator module explicitly. The problem is a developer's not going to know whether setParentFrom, startAndRun or startAndCall are used internally by other flows within the trace module, anything that calls those methods could potentially throw a runtime error.

you'll need to add it as an explicit dependency

The problem here is we get the reverse warning now, we get an "unused dependency" warning when we add the dependency I guess an unused dependency is the lesser of two evils, but it's still not an ideal state.

[Thrillpool]

I think the confusing bit here is just that Gradle has its opinions re optional dependencies https://blog.gradle.org/optional-dependencies.

Normally in this situation you might imagine https://repo1.maven.org/maven2/io/opentelemetry/opentelemetry-sdk-trace/1.47.0/opentelemetry-sdk-trace-1.47.0.pom having an optional dependency on incubator, but Gradle doesn't agree with this concept.

That being said, it does seem like https://repo1.maven.org/maven2/io/opentelemetry/opentelemetry-sdk-trace/1.47.0/opentelemetry-sdk-trace-1.47.0.module would ideally have this capabilities field, and maybe as a convenience to non Gradle people the pom could have a declared optional dependency?

[jack-berg]

From a quick investigation, looks like https://github.com/open-telemetry/opentelemetry-java/pull/6944 removes the incubator dependency from the classpath, but I see it's still being referenced in places like ExtendedSdkSpanBuilder

This is a strange error for sbt to generate. It implies that all compileOnly dependencies are invalid, since any code with a compileOnly dependency will have references to those classes in source code. The key bit we do is that we ensure that the classes from the compileOnly dependency are never referenced / loaded unless the user independently includes the incubator module.

The problem is a developer's not going to know whether setParentFrom, startAndRun or startAndCall are used internally by other flows within the trace module, anything that calls those methods could potentially throw a runtime error

For a runtime error to occur, the code that calls those methods would need to have a compileOnly dependency on the incubator, which would be strange. The more sensible thing would be for the code to have an implementation (or api) dependency on the incubator, which would cause the incubator to be on the classpath, which would avoid the runtime exception.

[trask]

I think the confusing bit here is just that Gradle has its opinions re optional dependencies https://blog.gradle.org/optional-dependencies.

@Thrillpool thanks for sharing that link!

I've attempted to use the gradle features concept in #7137