dotnet/runtime

Analyzer proposal: EventSource log argument guarding

Open

#45.215 aperta il 25 nov 2020

Vedi su GitHub
 (2 commenti) (0 reazioni) (0 assegnatari)C# (5445 fork)batch import
api-suggestionarea-System.Diagnostics.Tracingcode-analyzercode-fixerfeature-requesthelp wanted

Metriche repository

Star
 (17.886 star)
Metriche merge PR
 (Merge medio 12g 11h) (661 PR mergiate in 30 g)

Descrizione

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.

Guida contributor