-
Notifications
You must be signed in to change notification settings - Fork 263
Add logic to send logs through File Sink #2825
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
Conversation
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
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.
Pull Request Overview
This PR implements the logic to send logs from Data API Builder (DAB) to files using the File Sink feature with Serilog. It completes the File Sink functionality by adding the actual logging pipeline that writes logs to specified file paths.
Key changes include:
- Integration of Serilog file logging pipeline in the service configuration
- Removal of custom
RollingIntervalMode
enum in favor of Serilog's built-inRollingInterval
- Addition of comprehensive unit tests for File Sink functionality
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/Service/Startup.cs | Adds Serilog configuration and File Sink setup logic |
src/Service/Program.cs | Updates logger factory to include Serilog file logging |
src/Service/Azure.DataApiBuilder.Service.csproj | Adds Serilog package dependencies |
src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs | Adds comprehensive unit tests for File Sink functionality |
src/Directory.Packages.props | Defines Serilog package versions |
src/Config/ObjectModel/RollingIntervalMode.cs | Removes custom enum (deleted file) |
src/Config/ObjectModel/FileSinkOptions.cs | Updates to use Serilog's RollingInterval enum |
src/Config/Converters/FileSinkConverter.cs | Updates converter to use Serilog's RollingInterval |
src/Config/Azure.DataApiBuilder.Config.csproj | Adds Serilog dependency to config project |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
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.
Approved with suggesstions.
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
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.
Approved with some suggestions to add additional test checks
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
## Why make this change? - Closes issue #2578 - In order to complete the File Sink feature, the logic that sends the logs created by DAB to the file is required. ## What is this change? - This change implements the logic that sends all the logs from DAB to the file through the path the user requests. This is done with the help of Serilog which is a logging library that simplifies the creation of file sinks. - The `Startup.cs` program creates the Serilog logger pipeline and adds it as part of the services so that it is used later by the `Program.cs` to set the different loggers with the Serilog pipeline and allow the logs to be sent to the file sink. - We also deleted the `RollingIntervalMode.cs` since we discovered that Serilog has its own rolling interval enum class, which makes the one implemented in DAB obsolete. ## How was this tested? - [ ] Integration Tests - [X] Unit Tests Created tests that check if the services needed for the File Sink exist when the File Sink property is enabled. Also, created test to check if the file sink with the appropriate name is created when the property is enabled. ## Sample Request(s) <img width="917" height="136" alt="image" src="https://github.com/user-attachments/assets/a470844a-6ea3-4d05-9dfa-650f878c1c59" /> <img width="1873" height="948" alt="image" src="https://github.com/user-attachments/assets/1895eaae-0223-4d48-924e-c5275a98e3dc" />
## Why make this change? - Closes issue #2578 - In order to complete the File Sink feature, the logic that sends the logs created by DAB to the file is required. ## What is this change? - This change implements the logic that sends all the logs from DAB to the file through the path the user requests. This is done with the help of Serilog which is a logging library that simplifies the creation of file sinks. - The `Startup.cs` program creates the Serilog logger pipeline and adds it as part of the services so that it is used later by the `Program.cs` to set the different loggers with the Serilog pipeline and allow the logs to be sent to the file sink. - We also deleted the `RollingIntervalMode.cs` since we discovered that Serilog has its own rolling interval enum class, which makes the one implemented in DAB obsolete. ## How was this tested? - [ ] Integration Tests - [X] Unit Tests Created tests that check if the services needed for the File Sink exist when the File Sink property is enabled. Also, created test to check if the file sink with the appropriate name is created when the property is enabled. ## Sample Request(s) <img width="917" height="136" alt="image" src="https://github.com/user-attachments/assets/a470844a-6ea3-4d05-9dfa-650f878c1c59" /> <img width="1873" height="948" alt="image" src="https://github.com/user-attachments/assets/1895eaae-0223-4d48-924e-c5275a98e3dc" />
Why make this change?
What is this change?
Startup.cs
program creates the Serilog logger pipeline and adds it as part of the services so that it is used later by theProgram.cs
to set the different loggers with the Serilog pipeline and allow the logs to be sent to the file sink.RollingIntervalMode.cs
since we discovered that Serilog has its own rolling interval enum class, which makes the one implemented in DAB obsolete.How was this tested?
Created tests that check if the services needed for the File Sink exist when the File Sink property is enabled.
Also, created test to check if the file sink with the appropriate name is created when the property is enabled.
Sample Request(s)