Skip to content

FIX: Enrich context before collecting properties #149

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

Merged
merged 1 commit into from
Nov 8, 2019
Merged

FIX: Enrich context before collecting properties #149

merged 1 commit into from
Nov 8, 2019

Conversation

purplepangolin
Copy link

LogCompletion event was not including properties added via
EnrichDiagnosticContext delegate on options because
the properties had been collected before invoking the enricher.

Swapping the order of the statements fixes this.

This is a fix for #148

I'm afraid I haven't had time to add a test for this. I have built the package locally and tested it with the project I am currently working on.

LogCompletion event was not including properties added via
`EnrichDiagnosticContext` delegate on options because
the properties had been collected before invoking the enricher.

Swapping the order of the statements fixes this.
@nblumhardt
Copy link
Member

Oops! Nice catch, thanks 👍

@D9001
Copy link

D9001 commented Nov 11, 2019

Hi, I was following some blog posts and set up my app to use EnrichDiagnosticContext from within UseSerilogRequestLogging in a .NET Core 2.0 library. This made my app logging so much cleaner.
However, I just realized that I only have access to these when I use I use one of the latest dev versions of the nuget package. Is there going to be a stable release available soon which makes use of these? Just wanted to make sure before I roll back my changes in time for our production release.

@nblumhardt
Copy link
Member

@D9001 I'll set up a WIP release PR now so that you can track it, thanks; sounds like you're happy with the shape of EnrichDiagnostiContext as it's implemented in the previews?

@nblumhardt nblumhardt mentioned this pull request Nov 11, 2019
@D9001
Copy link

D9001 commented Nov 11, 2019

@nblumhardt Absolutely. Thanks!

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.

4 participants