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

Wrapped enrichers, conditional/leveled enrichers, conditional sinks #1316

Merged
merged 3 commits into from Jun 3, 2019

Conversation

nblumhardt
Copy link
Member

@nblumhardt nblumhardt commented May 29, 2019

What issue does this PR address?

This introduces some new configuration options, in part arising out of #1310.

  • LoggerEnrichmentConfiguration.Wrap(...) - mirrors LoggerSinkConfiguration.Wrap(), enabling enrichers to be wrapped; the two cases below illustrate its utility; it's public so that e.g. Serilog.Filters.Expressions could implement an expression-based Enrich.When().
  • Enrich.When(condition, configuration) - apply an enricher only to events that match condition; this will enable the requirement in Levelled enrichers/LoggerEnrichmentConfiguration.Wrap() #1310 to be met.
  • Enrich.AtLevel(level|switch, configuration) - apply an enricher only to events at or above a specified level (this differs from the semantics of AtLevel() discussed in the linked issue). I initially thought FromLevel() would be more consistent with Console()'s standardErrorFromLevel:, but the From prefix is confusing alongside Enrich.FromLogContext().
  • WriteTo.Conditional(condition, configuration) - write events matching condition to a sink. We already have the shortcut restrictedToMinimumLevel options on sink methods; this method is similar in spirit, and avoids an inefficient and verbose WriteTo.Logger(Filter...WriteTo) group. Also a difficult naming problem, I'd initially thought to call this When() to match the enricher equivalent, but the WriteTo.Noun pattern is distinct from Enrich.ActionPhrase pattern and using the same name here read strangely. Conditional() is the most noun-y option I could come up with, suggestions appreciated.

Of these, the enrichment changes enable possibilities that can't be achieved with Serilog 2.8.0, and I think they're more obviously a win for that.

WriteTo.Conditional() seems closer to borderline as it's essentially sugar; the efficiency gain (not having to allocate copied LogEvents along with their Properties dictionaries) tips the scales in favor of including it, IMO.

Opinions and suggestions most welcome! :-)

Does this PR introduce a breaking change?

No; minor version bump.

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:

@nblumhardt nblumhardt changed the title Wrapped enrichers Wrapped enrichers, conditional/leveled enrichers, conditional sinks May 29, 2019
@merbla
Copy link
Contributor

merbla commented Jun 1, 2019

🚢 IT

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.

None yet

2 participants