-
Notifications
You must be signed in to change notification settings - Fork 770
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
Fix EventSource for B3Propagator #3336
Fix EventSource for B3Propagator #3336
Conversation
/// This is used for internal logging of this library. | ||
/// </summary> | ||
[EventSource(Name = "OpenTelemetry.Extensions.Propagators")] | ||
internal class OpenTelemetryPropagatorsEventSource : EventSource |
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.
nit: internal sealed class
[EventSource(Name = "OpenTelemetry.Extensions.Propagators")] | ||
internal class OpenTelemetryPropagatorsEventSource : EventSource | ||
{ | ||
public static OpenTelemetryPropagatorsEventSource Log = new(); |
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.
nit:
public static readonly OpenTelemetryPropagatorsEventSource Log = new();
or
public static OpenTelemetryPropagatorsEventSource Log { get; } = new();
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.
LGTM
Both the nits should be applied across entire set of EventSource classes. Would make it a follow up PR. |
src/OpenTelemetry.Extensions.Propagators/OpenTelemetryPropagatorsEventSource.cs
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## main #3336 +/- ##
=======================================
Coverage 85.44% 85.44%
=======================================
Files 269 270 +1
Lines 9552 9560 +8
=======================================
+ Hits 8162 8169 +7
- Misses 1390 1391 +1
|
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.
Tiny correction needed but 👍
…orsEventSource.cs Co-authored-by: Alan West <[email protected]>
Own eventsource.
Also moved the public api files to unshipped, as the stable has not been released yet