Setup Correspondence with Serilog #652
Conversation
📝 WalkthroughWalkthroughThe pull request introduces Serilog logging capabilities to the Altinn Correspondence API project. It includes the addition of several new package references for Serilog in the project file and the configuration of Serilog in the Program.cs file. The implementation allows for logging to both console and Application Insights, with features for reading configuration settings and enriching logs with contextual information. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Altinn.Correspondence.API/Program.cs (1)
Line range hint
21-64: Add proper cleanup of Serilog resources.To ensure proper cleanup of logging resources, wrap the application startup in a try-catch block with Log.CloseAndFlush().
Example implementation:
static void BuildAndRun(string[] args) { + Log.Logger = new LoggerConfiguration() + .WriteTo.Console() + .CreateBootstrapLogger(); + + try + { var builder = WebApplication.CreateBuilder(args); builder.Host.UseSerilog((context, services, configuration) => configuration .ReadFrom.Configuration(context.Configuration) // ... rest of the configuration ... ); // ... rest of the setup ... app.Run(); + } + catch (Exception ex) + { + Log.Fatal(ex, "Application terminated unexpectedly"); + } + finally + { + Log.CloseAndFlush(); + } }
🧹 Nitpick comments (1)
src/Altinn.Correspondence.API/Program.cs (1)
24-31: Consider enhancing the Serilog configuration for production readiness.While the basic setup is good, consider these improvements for a more robust logging infrastructure:
- Set minimum log levels for different environments
- Add file sink for local logs
- Configure log retention policies
Example configuration:
builder.Host.UseSerilog((context, services, configuration) => configuration .ReadFrom.Configuration(context.Configuration) .ReadFrom.Services(services) .Enrich.FromLogContext() + .MinimumLevel.Information() + .MinimumLevel.Override("Microsoft", LogEventLevel.Warning) .WriteTo.Console() + .WriteTo.File( + path: "logs/correspondence-.log", + rollingInterval: RollingInterval.Day, + retainedFileCountLimit: 7) .WriteTo.ApplicationInsights( services.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Traces));
🛑 Comments failed to post (1)
src/Altinn.Correspondence.API/Altinn.Correspondence.API.csproj (1)
21-25: 💡 Codebase verification
Invalid version 4.0.0 specified for Serilog.Sinks.ApplicationInsights
The specified version 4.0.0 does not exist. The latest stable version available is 2.0.20. This needs to be corrected to ensure successful package restoration and builds.
🔗 Analysis chain
Verify compatibility of Serilog.Sinks.ApplicationInsights version.
While most Serilog packages are on version 9.0.0, the ApplicationInsights sink is on 4.0.0. Let's verify if this is the latest compatible version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest versions of Serilog.Sinks.ApplicationInsights and its compatibility # Get latest version from NuGet curl -s https://api.nuget.org/v3/registration5-semver1/serilog.sinks.applicationinsights/index.json | jq -r '.items[0].upper' # Check if there are any known compatibility issues gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NUGET, package: "Serilog.Sinks.ApplicationInsights") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 503
Script:
#!/bin/bash # Verify package information using nuget.org API curl -s "https://api.nuget.org/v3/registration5-semver1/serilog.sinks.applicationinsights/page/2.0.20/4.0.0.json" # Check if package exists in project's lock file fd -g "*packages.lock.json" --exec cat {}Length of output: 372
Script:
#!/bin/bash # Get all versions from NuGet curl -s "https://api.nuget.org/v3/registration5-semver1/serilog.sinks.applicationinsights/index.json" | jq -r '.items[0].items[].catalogEntry.version'Length of output: 641
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Altinn.Correspondence.API/Program.cs (1)
19-21: Consider enhancing bootstrap logger configuration.While the bootstrap logger is correctly configured, it could benefit from minimal error handling capabilities during startup.
Consider enhancing the bootstrap logger:
Log.Logger = new LoggerConfiguration() .WriteTo.Console() + .MinimumLevel.Warning() + .Enrich.FromLogContext() .CreateBootstrapLogger();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Altinn.Correspondence.API/Program.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
src/Altinn.Correspondence.API/Program.cs (2)
12-12: LGTM! Required imports added for Serilog integration.The necessary imports for Serilog and Application Insights have been correctly added.
Also applies to: 16-16
22-33: LGTM! Proper exception handling with logging.The try-catch-finally block correctly handles application termination:
- Logs fatal errors during unexpected termination
- Ensures proper cleanup of logging resources
Description
Setup Correspondence with Serilog in same way as Broker to make logs structured.
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit