CertificateManagerEventSource Has Incorrectly Authored Events
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
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 an`EventListener` 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.
Contributor guide
Assessment
This issue has not been assessed yet.