-
Notifications
You must be signed in to change notification settings - Fork 861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spec compliance: OTEL_PROPAGATORS should still work when OTEL_SDK_DISABLED #7062
Spec compliance: OTEL_PROPAGATORS should still work when OTEL_SDK_DISABLED #7062
Conversation
b4d723e
to
1660f64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you see if you can add a test for this?
3bc9876
to
1829e72
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7062 +/- ##
============================================
- Coverage 89.84% 89.83% -0.01%
- Complexity 6610 6612 +2
============================================
Files 740 740
Lines 19981 19983 +2
Branches 1966 1966
============================================
+ Hits 17951 17952 +1
+ Misses 1440 1439 -1
- Partials 590 592 +2 ☔ View full report in Codecov by Sentry. |
137ee88
to
198625e
Compare
...ure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
Outdated
Show resolved
Hide resolved
Interesting. I feel like historically class loaders are pretty hard to test
properly. This test was an attempt in vain to get the code coverage over
90%. Should I just strike it as it’s quite out of scope for this PR?
…On Thu, Feb 6, 2025 at 15:15 jack-berg ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
<#7062 (comment)>
:
> + () ->
+ AutoConfiguredOpenTelemetrySdk.builder()
+ .addPropertiesSupplier(() -> singletonMap("otel.experimental.config.file", ""))
+ .addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
+ .build());
+ }
+
+ @test
+ @SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
+ void serviceClassLoader() {
+ ClassLoader classLoader = null;
+ assertThatThrownBy(
+ () -> AutoConfiguredOpenTelemetrySdk.builder().setServiceClassLoader(classLoader))
+ .isInstanceOf(NullPointerException.class)
+ .hasMessageContaining("serviceClassLoader");
+ ClassLoader mockClassLoader = mock(ClassLoader.class);
I think this is what's causing the test to fail: mockito/mockito#1696
(comment)
<mockito/mockito#1696 (comment)>
I can recreate the test failure locally on my machine.
—
Reply to this email directly, view it on GitHub
<#7062 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5ZPN2VN5N3FFVKPR45P2D2OPGINAVCNFSM6AAAAABWJNNAG2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOJZHE3TENZQGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yeah I think that's the simplest path. |
249232f
to
53cd7fe
Compare
53cd7fe
to
90a58ef
Compare
No description provided.