Bump the Otel SDK and Otel Semantic Conventions version#37717
Bump the Otel SDK and Otel Semantic Conventions version#37717terrymanu merged 1 commit intoapache:masterfrom
Conversation
5aaae93 to
9ea633c
Compare
There was a problem hiding this comment.
Pull request overview
This pull request bumps the OpenTelemetry SDK and Semantic Conventions versions to address issue #37683. The changes modernize dependency management by migrating from explicit version declarations to BOM (Bill of Materials) imports for better dependency consistency.
Changes:
- Updated OpenTelemetry SDK from 1.41.0 to 1.58.0
- Updated OpenTelemetry Semantic Conventions from 1.27.0-alpha to 1.37.0
- Migrated from explicit dependency versions to BOM-based dependency management for Kotlin and OkHttp
- Added additional Kotlin stdlib exclusion for the okhttp dependency
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
terrymanu
left a comment
There was a problem hiding this comment.
The direction to bump OTel SDK/semconv is right and scoped to the OTel tracing POM—nice job keeping the change contained.
Change requests:
- Dependency correctness: com.squareup.okhttp3:okhttp at 5.3.2 is just a metadata artifact and contains no classes. Build is green only because OTLP exporter currently pulls in okhttp-jvm; meanwhile you exclude okhttp from Zipkin exporter. If OTLP is trimmed or shade filters shift, Zipkin export can hit NoClassDefFoundError. Please switch to an explicit okhttp-jvm dependency (or drop the Zipkin okhttp exclusion so its 4.12.0 comes through) and rebuild.
- Validation gap: The upgrade jumps OTel 1.41→1.58, semconv 1.27-alpha→1.37, Kotlin 1.9→2.2, OkHttp 4→5. After fixing the dependency, run at least the module build/basic smoke to confirm the shaded agent carries the correct okhttp classes and no conflicts.
No unrelated changes spotted. Please address the okhttp dependency and add a quick verification note; happy to re-review after that.
linghengqian
left a comment
There was a problem hiding this comment.
If OTLP is trimmed or shade filters shift, Zipkin export can hit NoClassDefFoundError. Please switch to an explicit okhttp-jvm dependency (or drop the Zipkin okhttp exclusion so its 4.12.0 comes through) and rebuild.
- I don't understand. What's the relationship between Zipkin and okhttp 4.12.0? Are there any background CI logs or issues?
com.squareup.okhttp3:okhttp-jvm:5.3.2is a transitive dependency ofio.opentelemetry:opentelemetry-bom:1.58.0withinio.opentelemetry:opentelemetry-exporter-sender-okhttp:1.58.0.
ee5f017 to
f6fcecd
Compare
f6fcecd to
181bf5d
Compare
There was a problem hiding this comment.
Dependency correctness: com.squareup.okhttp3:okhttp at 5.3.2 is just a metadata artifact and contains no classes. Build is green only because OTLP exporter currently pulls in okhttp-jvm; meanwhile you exclude okhttp from Zipkin exporter.
So what you're talking about is actually square/okhttp#8913 .
[INFO] --- enforcer:3.2.1:enforce (enforce-banned-dependencies) @ shardingsphere-agent-tracing-opentelemetry ---
[INFO] Recompiling the module because of changed dependency.
[INFO] Compiling 11 source files with javac [debug release 8] to target/test-classes
Error:
Dependency convergence error for com.squareup.okio:okio-jvm:jar:3.16.4 paths to dependency are:
+-org.apache.shardingsphere:shardingsphere-agent-tracing-opentelemetry:jar:5.5.3-SNAPSHOT
+-io.opentelemetry:opentelemetry-exporter-otlp:jar:1.58.0:compile
+-io.opentelemetry:opentelemetry-exporter-sender-okhttp:jar:1.58.0:runtime
+-com.squareup.okhttp3:okhttp-jvm:jar:5.3.2:runtime
+-com.squareup.okio:okio-jvm:jar:3.16.4:runtime
and
+-org.apache.shardingsphere:shardingsphere-agent-tracing-opentelemetry:jar:5.5.3-SNAPSHOT
+-com.squareup.okhttp3:okhttp:jar:4.12.0:compile
+-com.squareup.okio:okio:jar:3.6.0:compile
+-com.squareup.okio:okio-jvm:jar:3.6.0:compile
But I still don't understand why opentelemetry-exporter-zipkin was initially included. opentelemetry-exporter-zipkin is deprecated and unmaintained. It doesn't even align with the dependencies of opentelemetry-java, leading to the conclusion that everyone should only use OTLP (see open-telemetry/opentelemetry-specification#4715 and open-telemetry/opentelemetry-java#7863 ). Incidentally, zipkin also supports OTLP (see https://github.com/openzipkin-contrib/zipkin-otel/blob/main/module/README.md ). I believe we should remove the use of opentelemetry-exporter-zipkin in the future, and a new issue for this has been opened: #37723 .
linghengqian
left a comment
There was a problem hiding this comment.
After fixing the dependency, run at least the module build/basic smoke to confirm the shaded agent carries the correct okhttp classes and no conflicts.
- Done.
…ngsphere/master * remotes/origin/master: Add SQL parser test cases for SQL92 (apache#37727) Bump the Otel SDK and Otel Semantic Conventions version (apache#37717)
Fixes #37683 .
Changes proposed in this pull request:
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.