feat: add Serilog logging instrumentation #1110
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement comprehensive logging infrastructure using Serilog: - Add LoggingConfig for configurable log level, file output, JSON format - Add SerilogFactory to create Microsoft.Extensions.Logging.ILoggerFactory - Add LoggerExtensions with CallerMemberName-based method entry/exit logging - Add ActivityEnricher for correlation IDs (TraceId/SpanId) - Add SensitiveDataScrubbingPolicy that redacts strings in Production - Add EnvironmentDetector for environment-based security decisions - Add LoggingConstants with centralized magic values - Integrate LoggingConfig into AppConfig - Add TestLoggerFactory for XUnit test output visibility Features: - Console output to stderr (Warning+ by default) - File logging with daily rotation, 100MB limit, 30 file retention - JSON format option for log aggregation - Async logging option for services - Automatic sensitive data scrubbing in Production environment Test coverage: 84.83% (95 new tests)
Wire Serilog logging to CLI commands: - Add --log-file option to enable file logging from CLI - Wire --verbosity to Serilog log levels (silent=Fatal, quiet=Warning, normal=Info, verbose=Debug) - Inject ILoggerFactory via DI into all commands - Update BaseCommand to accept ILoggerFactory - Update all command constructors to pass logger factory - Fix environment detector test isolation
There was a problem hiding this comment.
Pull request overview
Implements comprehensive Serilog-based logging infrastructure for the application, enabling configurable logging to console (stderr) and files with automatic sensitive data scrubbing in Production environments.
Key Changes:
- Added Serilog logging components:
SerilogFactory,LoggingConfig,ActivityEnricher,SensitiveDataScrubbingPolicy,EnvironmentDetector, andLoggerExtensions - Integrated logging into CLI commands via dependency injection with
ILoggerFactoryparameter - Added CLI flags (
--verbosity,--log-file) and configuration file support for logging settings
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/Core/Logging/*.cs |
New logging infrastructure classes |
src/Core/Config/AppConfig.cs |
Added Logging property for configuration |
src/Main/CLI/CliApplicationBuilder.cs |
Integrated logger factory creation and DI registration |
src/Main/CLI/Commands/*.cs |
Updated all commands to accept ILoggerFactory parameter |
src/Main/CLI/GlobalOptions.cs |
Added log-file option and GetLogLevel() method |
tests/Core.Tests/Logging/*.cs |
Comprehensive test coverage for logging components |
tests/Main.Tests/**/*.cs |
Updated tests to pass NullLoggerFactory.Instance |
**/Directory.Packages.props |
Added Serilog package dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add EmptyTraceId and EmptySpanId constants to LoggingConstants - Use constants in ActivityEnricher instead of magic strings - Fix ParseArgValue loop to check all arguments including last one - Document ILoggerFactory disposal behavior in Build() remarks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements comprehensive logging infrastructure using Serilog (Feature #6):
LogMethodEntry/LogMethodExithelpersFeatures
Documentation
docs/CONFIGURATION.mdwith complete configuration referenceconfiguration-example.jsonwith logging sectionTest Plan
format.shpassesbuild.shpasses with 0 warnings, 0 errorscoverage.shpasses