- Notifications
You must be signed in to change notification settings - Fork327
Add Serilog example#107
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
TGrannen commentedFeb 4, 2024
@dotnet-policy-service agree |
| builder.Services.AddSerilog((_, loggerConfiguration) => | ||
| { | ||
| // Configure Serilog as desired here for AppHost logs (or use IConfiguration) | ||
| loggerConfiguration | ||
| .ReadFrom.Configuration(builder.Configuration) | ||
| .MinimumLevel.Information() | ||
| .MinimumLevel.Override("Aspire.Hosting.Dcp", LogEventLevel.Warning) | ||
| .Enrich.FromLogContext() | ||
| .WriteTo.Console(); | ||
| }); |
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.
Though I've seen 1 person ask. I'm not sure what the point of pushing logs the AppHost logs through serilog.
| builder.Services.AddSerilog((_,loggerConfiguration)=> | |
| { | |
| // Configure Serilog as desired here for AppHost logs (or use IConfiguration) | |
| loggerConfiguration | |
| .ReadFrom.Configuration(builder.Configuration) | |
| .MinimumLevel.Information() | |
| .MinimumLevel.Override("Aspire.Hosting.Dcp",LogEventLevel.Warning) | |
| .Enrich.FromLogContext() | |
| .WriteTo.Console(); | |
| }); |
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.
Ya, I also saw it in one of the comments and figured I'd include it. But I'm not really sure why it would be super useful since the AppHost is really just wiring everything up.
I also included this block in the Readme if it makes more sense to just have it as a reference.
| /// ⚠ This method MUST be called before the <see cref="OpenTelemetryLoggingExtensions.AddOpenTelemetry(ILoggingBuilder)"/> method to still send structured logs via OpenTelemetry. ⚠ | ||
| /// </remarks> |
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.
Why not disable the otlp logger in service defaults with a comment?
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 oltp logger still needs to be enabled and configured to get Structured Logs to work in the dashboard (instead of just console logs). I attempted to use the Serilog OpenTelemetry Sink instead of the oltp logger, but I ran into theissue mentioned earlier today that ended up creating multiple records in the dashboard. It would be much better to switch to Serilog entirely, but I didn't know exactly what OpenTelemetry/Sink configuration might need to be added to prevent the issue in the dashboard. I've only lightly dabbled with OpenTelemetry so far and didn't have time to get too far into it on a Saturday night.
If there's a simple fix for the issue or somewhere that I could go to to get pointed down the right path, I'd be happy to try a few more things and rework this to switch everything over to just use Serilog.
I also mentioned this same thing inmy comment on the issue related to this PR.
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.
Is this PR trying to showing using serilog alongside the otel logger provider?
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.
That's correct, but more as a work around than what the final desired approach would be.
Example for logging with Serilog#106