Skip to content
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

Add new mechanism to enable creation of DiagnosticContext scopes #49

Closed

Conversation

gkinsman
Copy link

@gkinsman gkinsman commented Jul 15, 2021

This PR is a WIP proposal to fix #47. There's still a few unanswered questions below as I'm not sure this is 100% the right direction :).

It introduces a new method to IDiagnosticContext that allows users to begin a new DiagnosticContextScope that collects properties via IDiagnosticContext:

DiagnosticContextScope Begin(string messageTemplate, params object[] properties);

Open Questions

There's still a number of areas that haven't been fully thought through on the interface method:

  • Configurable log levels (like with RequestLoggingMiddleware)?
  • Additional enrichers for the completion event?
  • Specify a custom logger to use?
  • It's possible IDiagnosticContext isn't the right place for Begin, and that maybe it belongs on some other thing that should be injected to keep IDC clean. Putting it on ILogger might work, but then you'd need to pass in an IDiagnosticContext.
  • Is adding this to IDiagnosticContext even an option? It's public so others could implement it, but there's no real way to use it from what I can tell.

Sample Usage

...
var operationId = Guid.NewGuid();
using (_diagnosticContext.Begin("Worker executed operation {OperationId} at time {Time}", operationId, DateTimeOffset.Now)) {
    await _executor.DoWork(operationId);
}
...

public async Task DoWork(Guid operationId)
{
    ...
    _diagnosticContext.Set("OperationName", randomOperation);
    _diagnosticContext.Set("ThisOperationId", operationId);
    ...
}
...

Which produces this event:
image

George Kinsman added 2 commits July 15, 2021 12:44
…at RequestLoggingMiddleware does

- Add new sample project to demonstrate its usage
- Add nested example to sample app
@nblumhardt
Copy link
Member

Thanks for sending this; will have a dig in properly as soon as I have a chance (the last few months have been a bit crazy, here :-))

@gkinsman
Copy link
Author

Thanks for sending this; will have a dig in properly as soon as I have a chance (the last few months have been a bit crazy, here :-))

No worries Nick! Hope everything's well 🙂

@nblumhardt
Copy link
Member

@gkinsman snowed :-D but digging out hehe 👍

@gkinsman gkinsman marked this pull request as ready for review September 28, 2021 12:52
@gkinsman gkinsman closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extensibility opportunity for IDiagnosticContext?
3 participants