Analyzer proposal: EventSource log argument guarding
#45.215 aberto em 25 de nov. de 2020
Métricas do repositório
- Stars
- (17.886 stars)
- Métricas de merge de PR
- (Mesclagem média 12d 11h) (661 fundiu PRs em 30d)
Description
Background and Motivation
The typical pattern with EventSource is to have an EventSource-derived type that exposes one method per event; arguments to these method calls are then generally included in some form as part of the event payload. With the goal of logging being a nop / as cheap as possible when it's not enabled, any expensive work should be guarded by checks for whether the EventSource is actually enabled, e.g.
if (MyEventSource.Log.IsEnabled())
{
MyEventSource.Log.SomethingInterestingHappened(ComputeArgument());
}
but it's easy to accidentally forget such a guard. https://github.com/dotnet/aspnetcore/pull/27956 has examples of cases in ASP.NET that we shipped in .NET 5, where unguarded log calls were doing fairly expensive work, e.g.
Log.DescribeFoundCertificates(ToCertificateDescription(matchingCertificates));
Proposed Analyzer
We won't be able to catch all such uses perfectly, but we should be able to flag many, with a reasonably low false positive rate, e.g. find calls to SomeEventSource.Log.SomeMethod, and if any argument isn't a field/local or some property access off of one (making the assumption that properties are cheap), then find if there's a SomeEventSource.Log.IsEnabled() check guarding it, and if there isn't, warn there should be. An auto-fixer could add one.