-
Notifications
You must be signed in to change notification settings - Fork 812
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 meaningful .toString
to NoopLogRecordProcessor
and DefaultOpenTelemetry
#5493
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #5493 +/- ##
=========================================
Coverage 91.33% 91.34%
- Complexity 4891 4893 +2
=========================================
Files 547 547
Lines 14410 14412 +2
Branches 1354 1354
=========================================
+ Hits 13161 13164 +3
Misses 865 865
+ Partials 384 383 -1
☔ View full report in Codecov by Sentry. |
.toString
to NoopLogRecordProcessor
.toString
to NoopLogRecordProcessor
and DefaultOpenTelemetry
|
||
@Override | ||
public String toString() { | ||
return "DefaultOpenTelemetry{" |
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.
Still trying to figure out how that is correct. TracerProvider and MeterProvider are always noop. Should I include them in toString
?
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.
I'd say no. This looks fine to me.
52cdc45
to
56de53a
Compare
56de53a
to
7b68022
Compare
A small improvement. The behavior is similar to
opentelemetry-java/context/src/main/java/io/opentelemetry/context/propagation/NoopTextMapPropagator.java
Lines 36 to 39 in 93bcdf2