Skip to content

Property in LogEvent should not be overridden by scope#272

Open
huoshan12345 wants to merge 4 commits into
serilog:devfrom
huoshan12345:bugfix/property-in-logevent-should-not-be-overridden-by-scope
Open

Property in LogEvent should not be overridden by scope#272
huoshan12345 wants to merge 4 commits into
serilog:devfrom
huoshan12345:bugfix/property-in-logevent-should-not-be-overridden-by-scope

Conversation

@huoshan12345

@huoshan12345 huoshan12345 commented Apr 1, 2025

Copy link
Copy Markdown

fixes #271

NOTE:
ExternalScopeProvider.ForEachScope processes scopes from outer to inner, which is the opposite of Serilog's scope traversal order. This fix implements ForEachScopeReversed and passes update: false to the method EnrichWithStateAndCreateScopeItem to ensure consistent behavior matching Serilog's expectations.

@nblumhardt

Copy link
Copy Markdown
Member

Thanks for sending this. Because the method is called for every log statement, I'm not sure the added cost of creating, populating, and iterating through an additional List will be acceptable. We might need to investigate alternative approaches to #289 instead 🤔

@huoshan12345

Copy link
Copy Markdown
Author

Thanks for sending this. Because the method is called for every log statement, I'm not sure the added cost of creating, populating, and iterating through an additional List will be acceptable. We might need to investigate alternative approaches to #289 instead 🤔

Thanks for reviewing the PR!
I'm glad you brought this up - I've been exploring solutions but haven't found a better approach yet.
By the way, I noticed you referenced #289, but I don't seem to have access to that issue. Is it a private issue perhaps?

@nblumhardt

Copy link
Copy Markdown
Member

Sorry! The PR I meant to refer to is #269.

…ons-logging into bugfix/property-in-logevent-should-not-be-overridden-by-scope

# Conflicts:
#	src/Serilog.Extensions.Logging/Extensions/Logging/SerilogLoggerProvider.cs
#	test/Serilog.Extensions.Logging.Tests/SerilogLoggerTests.cs
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.

Behavior Change: Property Value Overriding in LogEvents Since Serilog.Extensions.Logging 9.0.1

2 participants