-
Notifications
You must be signed in to change notification settings - Fork 2
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
Pare down the C# example a little #1
Conversation
|
||
ActivitySource ExampleWeatherServiceActivitySource = new("Example Weather Service"); | ||
var exampleActivitySource = new ActivitySource("Example.WeatherService"); |
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.
Convention for activity source names is normally namespace-style.
|
||
try |
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.
Catch block is redundant, since allowing the exception to escape Main()
will also produce a nonzero exit code, and have the side-effect of printing some useful output to stderr.
{ | ||
openTelemetryLoggerOptions.SetResourceBuilder( | ||
ResourceBuilder.CreateEmpty() | ||
.AddService(serviceName: builder.Environment.ApplicationName)); |
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.
Removed the example resource attributes; these are normally set through environment variables and other means (although adding them here is still valid).
|
||
app.MapGet("/{postcode}", (string postcode) => | ||
// Some important options to improve data quality | ||
openTelemetryLoggerOptions.IncludeScopes = true; |
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.
Don't need IncludeFormattedMessage
anymore, Seq will heuristically detect message templates.
{ | ||
return 1; | ||
} | ||
using var activity = exampleActivitySource.StartActivity("Look up forecast for postcode {Postcode}"); |
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.
The suppressed warning at the top of the file is because we want to pass an activity name explicitly here.
much better! |
This proposes a couple of possible updates for the C# example. I'll comment them all inline, they're definitely not all necessary, let me know what you think! :-)