dotnet/aspnetcore

CertificateManagerEventSource Has Incorrectly Authored Events

Open

#29 541 ouverte le 22 janv. 2021

Voir sur GitHub
 (3 commentaires) (1 réaction) (0 assignés)C# (10 653 forks)batch import
affected-very-fewarea-commandlinetoolsbugfeature-devcertshelp wantedinvestigateseverity-nice-to-have

Métriques du dépôt

Stars
 (37 933 stars)
Métriques de merge PR
 (Merge moyen 16j 9h) (258 PRs mergées en 30 j)

Description

CertificateManagerEventSource has some events that are incorrectly authored. ListCertificatesStart (https://github.com/dotnet/aspnetcore/blob/main/src/Shared/CertificateGeneration/CertificateManager.cs#L782) is one such example:

            [Event(1, Level = EventLevel.Verbose)]
            public void ListCertificatesStart(StoreLocation location, StoreName storeName)
            {
                if (IsEnabled())
                {
                    WriteEvent(1, $"Listing certificates from {location}\\{storeName}");
                }
            }

There are two arguments to the method, both of which are enums, but the call to WriteEvent only takes one payload argument of type string. When a tool attempts to decode the event, or anEventListener dispatches the event, it will treat the event as if it has two enums as payloads, even though the data payload does not match. The way to fix this is to make sure that the arguments to WriteEvent match the arguments to the event method itself.

Another example is CreateDevelopmentCertificateEnd (https://github.com/dotnet/aspnetcore/blob/main/src/Shared/CertificateGeneration/CertificateManager.cs#L861), which takes no arguments, but passes a string to WriteEvent. In cases like this, the event name should describe the context around when the event is logged, which I believe it does. The string that gets passed to WriteEvent will not be seen by an event consumer, but is allocated and logged into the file.

In general, any usage of string constants or string interpolation inside of an event method is incorrect. Context should either come from the event name itself or from the arguments, but don't feel required to make the strings "super clean" - it's more important to make the logging paths as low cost as possible.

Guide contributeur