Repository navigation
To remove cross module internal utility usage from trace propagators - #8955
NithinU2802 wants to merge 10 commits into
Conversation
Pull request dashboard statusWaiting on reviewers · refreshed 2026-10-20 07:02 UTC Review the latest changes. Status above doesn't look right?
|
…ving only api internal utils
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8955 +/- ##
=========================================
Coverage 90.43% 90.44%
- Complexity 10908 10919 +11
=========================================
Files 1015 1016 +1
Lines 29854 29864 +10
Branches 3901 3904 +3
=========================================
+ Hits 26998 27010 +12
Misses 1907 1907
+ Partials 949 947 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| * @param minLength the minimum length of the returned string | ||
| * @return the padded string, or {@code value} if it already meets the minimum length | ||
| */ | ||
| static String padLeft(String value, int minLength) { |
There was a problem hiding this comment.
I think this is only used in the opentelemetry-extension-trace-propagators module and tests, so let's go ahead and remove it from io.opentelemetry.api.internal.StringUtils.
There was a problem hiding this comment.
This method is also used here in SdkLoggerTest.java, Line 79
| * @param string the string to check | ||
| * @return {@code true} if {@code string} is {@code null} or empty | ||
| */ | ||
| static boolean isNullOrEmpty(@Nullable String string) { |
There was a problem hiding this comment.
I think this method is kind of silly. Better to just enumerate the string == null || string.isEmpty() condition in the places that use it.
| * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change | ||
| * at any time. | ||
| */ | ||
| final class TemporaryBuffers { |
There was a problem hiding this comment.
I think we should consider promoting this to a .impl. API since a second ThreadLocal increases the the allocation per thread when reuse is safe
There was a problem hiding this comment.
yeah ok, I will move this TemporaryBuffers to api.impl and update the imports accordingly to follow this approach which avoid additional ThreadLocal creation.
|
…hub.com/NithinU2802/opentelemetry-java into fix/trace-propagator-internal-utilities
… tests for common
This branch has not been deployed
2 participants
This PR is to resolves a part #6970 by updating the trace-propagators module’s dependency on API internal utility classes.
Changes
StringUtilsandTemporaryBuffersutilities.