search

serilog / serilog-extensions-logging

Serilog provider for Microsoft.Extensions.Logging
Apache License 2.0
307 stars 97 forks source link

Possibility to rename hard-coded "EventId" property #200

Open rickardp opened 2 years ago

rickardp commented 2 years ago

If I read the code correctly in SerilogLogger, there is only one property that is being set directly by the code in this library: the EventId property. Since this is a static concept in Microsoft.Extensions.Logging, it gets translated to general SeriLog properties in this class. The problem is on the SeriLog side, this clashes with the properties provided by the application (which it does not on the Microsoft.Extensions.Logging side). As this property is hard coded here it is difficult to change the behaviour if integrating with existing logging code, or if application developers expects to be able to use this property, which would prevent adoption of the LoggerMessage-style logging.

For some background, EventId is a well-known primary key in many of our services and not being able to use this name would mean a lot of migration and confusion (resulting in high probability of logging bugs). It is not an immediate issue as we are not using the event ID/Name in these log lines currently, but since Microsoft seems to recommend migrating to LoggerMessage where performance is important this will be a future source of bugs for us that would be nice to have a solution for.

Would it be possible to introduce the option to reconfigure the name for this property, or introduce a (possibly configurable) common prefix for all "system-defined" properties? As far as I can see, "EventId" is the only property that is explicitly hard coded in this library in normal cases, but I did not look beyond the SerilogLogger class so I may be wrong here.

nblumhardt commented 2 years ago

Thanks for the note, Rikard. This would be nice to enable, but we'd need to think about how we'd expose it through the higher-level APIs like UseSerilog().

If anyone has a chance to look into it, a proposal for introducing an overload of UseSerilog() that accepts a trailing SerilogLoggingOptions object carrying parameters like WriteToProviders and PreserveStaticLogger would set the groundwork for this kind of change.