-
Notifications
You must be signed in to change notification settings - Fork 0
#13 - Part 1 - Add Serilog to save structured logs #22
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
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 adds Serilog structured logging to a Temporal workflow application. The changes introduce structured JSON logging to the /logs folder, which is mounted as a Docker volume for easy access.
- Integrates Serilog for structured logging with JSON output to files and console
- Updates namespace from
workflowstoWorkflowsacross the codebase - Modifies activities to use dependency injection for logging instead of static methods
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| temporal-examples/workflows/WaitingSignal.workflow.cs | Updates namespace and activity execution pattern |
| temporal-examples/workflows/Examples.workflow.cs | Updates namespace and activity execution pattern |
| temporal-examples/workflows/ExampleWithChildren.workflow.cs | Updates namespace and activity execution pattern |
| temporal-examples/workflows/ExampleActivities.cs | Adds constructor injection for logging and converts static methods to instance methods |
| temporal-examples/temporal-worker/temporal-worker.csproj | Adds Serilog package dependencies |
| temporal-examples/temporal-worker/Program.cs | Updates LoggingModule registration to pass configuration |
| temporal-examples/temporal-worker/AutoFac/Modules/TemporalWorkerConfigurationModule.cs | Updates namespace import and simplifies configuration assignment |
| temporal-examples/temporal-worker/AutoFac/Modules/LoggingModule.cs | Replaces custom logging with Serilog configuration for JSON file and console output |
| temporal-examples/docker-compose.yml | Adds volume mount for logs directory |
| temporal-examples/RestController/appsettings.Development.json | Adds Temporal configuration settings |
| temporal-examples/RestController/SignalController.cs | Updates namespace import |
| temporal-examples/Directory.Packages.props | Adds Serilog package versions and reformats existing entries |
Comments suppressed due to low confidence (1)
temporal-examples/workflows/ExampleActivities.cs:9
- The parameter name 'logg' is ambiguous and inconsistent with typical naming conventions. It should be renamed to 'logger' to match the field name '_logger'.
public ExampleActivities(ILogger<ExampleActivities> logg)
| { | ||
| await Task.Delay(2000); | ||
| var duration = 2000; | ||
| _logger.LogInformation($"Starting delay: {@duration}"); |
Copilot
AI
Aug 6, 2025
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 structured logging syntax is incorrect. The '@' symbol should be used without string interpolation. Change to '_logger.LogInformation("Starting delay: {Duration}", duration);' for proper structured logging.
| _logger.LogInformation($"Starting delay: {@duration}"); | |
| _logger.LogInformation("Starting delay: {Duration}", duration); |
| { | ||
| childWorkflowsInfo.Add($"child-{index}-workflow-{DateTime.Now}"); | ||
| var name = $"child-{index}-workflow-{DateTime.Now}"; | ||
| _logger.LogInformation($"Creating child workflow {@name}"); |
Copilot
AI
Aug 6, 2025
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 structured logging syntax is incorrect. The '@' symbol should be used without string interpolation. Change to '_logger.LogInformation("Creating child workflow {Name}", name);' for proper structured logging.
| _logger.LogInformation($"Creating child workflow {@name}"); | |
| _logger.LogInformation("Creating child workflow {@Name}", name); |
|
|
||
| string result = await Workflow.ExecuteActivityAsync( | ||
| () => ExampleActivities.GenericTask(), | ||
| (ExampleActivities a) => a.GenericTask(), |
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.
Whats the purpose of changing the syntax here? I feel as though both are readable as each other
Structured logs are saved in /logs folder, which is mounted as a volume in Docker making them easily accessible.
This doesn't yet integrate spans and trace IDs