Allow customization of default properties collected by UseSerilogRequestLogging()

[vhatsura]

I suggest to change Elapsed to ElapsedMilliseconds by the following reasons:

1. In case of using structured logging, you need to look into message to understand the value is specified in milliseconds.
2. It will be aligned with other places: HttpClient logs, default AspNetCore logs

[nblumhardt]

Hi Vadim, thanks for the suggestion.

Right now I think making this change would lead to a lot of downstream churn in monitoring systems and so-on.

I agree that the *Milliseconds name would be preferable given the other examples you've listed, but I think it's a ship that's already sailed at this point.

[dzmitry-lahoda]

Just bump Major version of lib. My life is miserable as all these people do name things each time differently. I am suffering.

[sungam3r]

This is nothing more than a conflict of subjective opinions. For example, I use Elapsed in my applications. After the change you proposed, should I suffer?

[vhatsura]

This is nothing more than a conflict of subjective opinions. For example, I use Elapsed in my applications. After the change you proposed, should I suffer?

I would say the main reason isn't in suffering. The main reason is the difference with the name of the property in Asp.Net Core logs and HttpClient. For new applications, it doesn't matter how the field is named (except misunderstanding in which units), however, for old applications that want to use Serilog request logging, it can be an issue due to configured patterns in structured logging storages in a different way. But it's also a tricky case for people who already switched to the new property names and reconfigured infrastructure. I would say that it will be awesome to have a change in the future major version of Serilog as it was done in .Net Core (https://github.com/aspnet/Announcements/issues/396). @nblumhardt, WDYT? Is it possible to change the property name in the future major version?

[nblumhardt]

Thanks for the follow-up @vhatsura 👍

Right now I still expect the pain caused by a change (especially given that large numbers of monitoring systems work with these events) will be too great to justify it. In particular, large enterprise deployments will have many services using the old and new versions of any package concurrently, making monitoring much harder.

Given that we support customization of MessageTemplate already, through RequestLoggingOptions, it does seem that adding another option there, Func<HttpContext, double, int, IEnumerable<LogEventProperty>> GetMessageTemplateProperties, could be worth exploring?

It would fit in at: https://github.com/serilog/serilog-aspnetcore/blob/dev/src/Serilog.AspNetCore/AspNetCore/RequestLoggingMiddleware.cs#L88

The default would just do the same array construction that we currently do inline, but an override could substitute property names etc.

We'd document it as returning one property per property token in the supplied message template - and this would be nice and easy for consumers to check by eye.

What do you think?

[dnperfors]

Allowing customization of these properties would be the preferred way. We also rely on ElapsedInMiliseconds for our monitoring, and therefor we can't use this middleware as efficient.