Fix StringEncoder javadoc Java version and Android condition - #8711
Fix StringEncoder javadoc Java version and Android condition#8711thswlsqls wants to merge 1 commit into
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-08-14 16:27 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8711 +/- ##
=========================================
Coverage 91.65% 91.65%
Complexity 10352 10352
=========================================
Files 1003 1003
Lines 27210 27210
Branches 3199 3199
=========================================
Hits 24939 24939
Misses 1566 1566
Partials 705 705 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| * <li>{@code UnsafeStringEncoder} - High-performance Java 8+ implementation using | ||
| * sun.misc.Unsafe. Only attempted when the Java version is detected to be below 23, to avoid | ||
| * the JEP 498 deprecation warning. | ||
| * <li>{@code UnsafeStringEncoder} - High-performance Java 9+ implementation using sun.misc.Unsafe |
There was a problem hiding this comment.
But the UnsafeStringEncoder is java 8+ 🤔
There was a problem hiding this comment.
@jack-berg Thanks for the review. This encoder is never selected on Java 8: createIfAvailable() requires String.value to be a byte[] and String.coder to exist, both compact-strings fields, and Java 8's String.value is a char[] — so it returns null. The existing comment on the coder lookup says the same thing.
You're right that the class itself compiles for Java 8; the doc sentence is about which runtimes actually get it. Happy to word it that way instead.
There was a problem hiding this comment.
After looking closer, you are correct, and this indeed is reflected in unit tests: https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/common/src/test/java/io/opentelemetry/exporter/internal/marshal/StringEncoderTest.java#L38
cc @trask
Fixes #8709
Description
UnsafeStringEncodera "Java 8+" implementation, butcreateIfAvailable()needs theString.value(byte[]) andString.coderfields, which Java 8 does not have, so Java 8 always falls back. The inline comment on thecoderlookup already says "this only exists in Java 9+".createUnsafeEncoder(), but the bullet still listed only the Java version condition. Added it to the same sentence.UnsafeStringEncoderjavadoc and theStringEncoderHolderlog message.Testing done
Level.FINElog message; no behavior or API change../gradlew :exporters:common:checkpassed — 70 tests (61test, 8testSenderProvider, 1testWithoutUnsafe), 0 failures.io.opentelemetry.exporter.internal.*.