-
Notifications
You must be signed in to change notification settings - Fork 858
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
Add experimental option to disable autoconfigured opentelemetry SDK #4489
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4489 +/- ##
============================================
- Coverage 90.18% 90.05% -0.14%
- Complexity 5010 5052 +42
============================================
Files 569 580 +11
Lines 15440 15562 +122
Branches 1488 1494 +6
============================================
+ Hits 13925 14014 +89
- Misses 1056 1087 +31
- Partials 459 461 +2
Continue to review full report at Codecov.
|
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.
Seems reasonable. Thanks!
...nfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdk.java
Outdated
Show resolved
Hide resolved
/** | ||
* Returns the {@link OpenTelemetrySdk} that was auto-configured, or {@code null} if the SDK has | ||
* been disabled. | ||
*/ | ||
public abstract OpenTelemetrySdk getOpenTelemetrySdk(); |
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.
This PR does not appear to have or add a code path that can return null
, and after several hours of poking at the current code, that fact does not seem to have changed since. Is this a bug? or did the design get changed after the doc was written?
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.
Oops. The response of this method was originally nullable but per this conversation, we went in a different direction. Looks like the javadoc never got updated - sorry about the confusion.
I've opened #6109 to fix the comment.
Resolves #4483.