elsa-workflows / elsa-workflows/elsa-core
[ENH] Enhance logging by adding logger state to activity and workflow pipelines
- Dominant language
- C#
- Stars
- 7.9k
- Forks
- 1.5k
- Avg merge
- 15h 22m
- Merged PRs (30d)
- 114
Description
## Enhancement Request
### Enhancement Overview
When viewing application logs, it is often useful to see contextual properties to help understand the logs. For instance, when viewing a log message it might be useful to know the workflow instance Id of the workflow being executed, or the activity type of the current activity. Adding this sort of context is usually accomplished through the use of the `ILogger.BeginScope` method. The desired format of the logger state is often very specific to the application. For instance, we use structured logging (serilog/datadog), and we might like to see the logger state nested like
```
workflow
/ name
/ instance_id
/ activity
/ id
/ type
````
but of course everyone would not like it like that... so the logger state generation should be injectable.
### Proposed Enhancement
I propose that new middleware be added to Elsa, both for the workflow pipeline and activity pipeline. The new middleware would make use of an injectable `ILoggerStateGenerator` (name TBD) which would return the logger state for the given context object. The default implementation may just add `WorkflowInstanceId` / `ActivityId` properties, where as a custom implementation could be injected if requried.
Note: `WorkflowInstanceId` logger state is currently added directly by `WorkflowRunner.RunAsync` - this should be removed in preference to the proposed middleware if we end up going with this proposal.
### Alternative Solutions
Although I believe new pipeline middleware would be the cleanest approach, it would also be possible to have the code which triggers the pipeline execution set the logger state around the pipeline execution. As I see it, this would be in `WorkflowRunner.RunAsync` for workflow scope (in place of there it already adds the `WorkflowInstanceId` logger state) and `ActivityInvoker.InvokeAsync` for activity scope.
### Additional Context
We currently have this implemented using custom middleware and would be happy to contribute this to Elsa. Or, if the alternative solution is preferred, we'd be happy to do this work too.
Contributor guide
Assessment
This issue has not been assessed yet.