Visitar URL original
Property factory as AddPropertyIfAbsent parameter by vanni-giachin · Pull Request #2149 · serilog/serilog · GitHub
Skip to content

Property factory as AddPropertyIfAbsent parameter - #2149

Merged
nblumhardt merged 1 commit into
serilog:devfrom
vanni-giachin:optimized-add-property-if-absent
Dec 29, 2024
Merged

Property factory as AddPropertyIfAbsent parameter#2149
nblumhardt merged 1 commit into
serilog:devfrom
vanni-giachin:optimized-add-property-if-absent

Conversation

@vanni-giachin

@vanni-giachin vanni-giachin commented Dec 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

In this PR I propose to set public this internal method overload:

LogEvent.AddPropertyIfAbsent(ILogEventPropertyFactory factory, string name, object? value, bool destructureObjects = false)

The goal is allow to write optimised ILogEventEnricherclasses and also to be able to use ILogEventPropertyFactory to create properties.

In therm of performance this solution would be almost the same as the code suggested in #739 where properties are created only once, but with the benefit of using the property factory as intended by the ILogEventEnricher interface.

Example:

class SampleEnricher(Foo foo) : ILogEventEnricher
{
    public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
    {
        // a LogEventProperty object is created by the factory only if missing
        logEvent.AddPropertyIfAbsent(propertyFactory, nameof(foo.Bar), foo.Bar);
    }
}

@vanni-giachin vanni-giachin mentioned this pull request Dec 19, 2024
Closed
@sungam3r

sungam3r commented Dec 24, 2024

Copy link
Copy Markdown
Contributor

As I can see from codebase PropertyEnricher allows you to call AddPropertyIfAbsent without creating LogEventProperty.

@vanni-giachin

vanni-giachin commented Dec 27, 2024

Copy link
Copy Markdown
Contributor Author

As I can see from codebase PropertyEnricher allows you to call AddPropertyIfAbsent without creating LogEventProperty.

@sungam3r yes, correct, PropertyEnricher calls the method I propose to turn public. In some scenarios I prefer to use a custom ILogEventEnricher since I have to enrich the log events with multiple correlated properties, like multiple fields of an object passed to the enricher constructor, that don't need to be recalculated every time a log is emitted.

@sungam3r

sungam3r commented Dec 27, 2024

Copy link
Copy Markdown
Contributor

When workflow run you'll see one failed test. Run api approval test locally.

@vanni-giachin
vanni-giachin force-pushed the optimized-add-property-if-absent branch from aee2ec1 to 304d744 Compare December 28, 2024 12:02
@vanni-giachin

vanni-giachin commented Dec 28, 2024

Copy link
Copy Markdown
Contributor Author

When workflow run you'll see one failed test. Run api approval test locally.

Ok. Fixed, thanks!

nblumhardt
nblumhardt approved these changes Dec 29, 2024
View reviewed changes

@nblumhardt nblumhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

@nblumhardt
nblumhardt merged commit e4e0b02 into serilog:dev Dec 29, 2024
@vanni-giachin
vanni-giachin deleted the optimized-add-property-if-absent branch February 4, 2025 21:19
@nblumhardt nblumhardt mentioned this pull request May 13, 2025
Merged
This was referenced Jul 22, 2025
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jul 29, 2025
Closed
Closed
Merged
Closed
Closed
This was referenced Sep 21, 2026
Closed
Closed
Merged
Closed
This was referenced Sep 29, 2026
Merged
Closed
Open
This was referenced Oct 9, 2026
Closed
Closed
Closed
Open
Closed
This was referenced Oct 16, 2026
Merged
Closed
Merged
Open
This was referenced Oct 24, 2026
Open
Closed
Closed
Open
Closed
Open
Open
This was referenced Oct 31, 2026
Closed
Merged
Open
Open
Closed
Merged
@dependabot dependabot Bot mentioned this pull request Nov 10, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nblumhardt nblumhardt nblumhardt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vanni-giachin @sungam3r @nblumhardt