Introduce a logger provider that counts warnings and errors #54846
Conversation
...if an `IMeterFactory` is present (and does nothing otherwise). This will provide insight into things like how often unknown key identifiers are encountered.
|
I'm almost certain the names are wrong but, before I fix that, I wanted to check whether this approach even made sense. |
src/DataProtection/DataProtection/src/DataProtectionServiceCollectionExtensions.cs
Show resolved
Hide resolved
| internal sealed class MetricsLoggerProvider : ILoggerProvider | ||
| { | ||
| public const string MeterName = "Microsoft.AspNetCore.DataProtection"; | ||
| private const string CategoryNamePrefix = "Microsoft.AspNetCore.DataProtection"; |
There was a problem hiding this comment.
Should I include a dot at the end? It's a namespace we "own", so I'm not that worried about picking up, e.g., "DataProtection2".
|
@noahfalk might have thoughts on how this plays with the other kinds of events in the dotnet ecosystem. |
I have seen other people interested in converting error/warning logs to metrics, but I am not aware that anyone has built one yet. I think the main thing to decide is if this is building a general-purpose mechanism or a specialized one just for DataProtection logs? If you built a special purpose one maybe it would eventually become obsolete in the future if a general-purpose option is created. |
| _meter = meterFactory.Create(MeterName); | ||
|
|
||
| var counter = _meter.CreateCounter<long>( | ||
| "aspnetcore.data_protection.log_messages", |
There was a problem hiding this comment.
We went with "rate_limiting", so I went with "data_protection", rather than "dataprotection".
|
|
||
| var tags = new TagList( | ||
| [ | ||
| new("aspnetcore.data_protection.log_message_id", eventId.Name ?? eventId.Id.ToString(CultureInfo.InvariantCulture)), |
There was a problem hiding this comment.
Arguably, these should be dotnet- or Microsoft.Extensions- scoped, but that seems premature.
There was a problem hiding this comment.
Just personal thoughts, maybe use "aspnetcore.data_protection.logs.event_name" and "aspnetcore.data_protection.logs.log_level"
It definitely seems like a general purpose need, but I guess I'd be interested in seeing how it plays out in a specific scenario before making a shared one. Personally, I was going to scope it to exactly DataProtection, but @JamesNK may have thoughts on whether it makes more sense to cover all of AspNetCore? My feeling was that this will generate relatively little clutter for consumers if we have to keep producing it forever after it becomes obsolete. |
|
No opposition from me if you want to make this one specific to DataProtection :) |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
dotnet-policy-service
bot
added
the
pending-ci-rerun
Co-authored-by: Reiley Yang <[email protected]>
...if an
IMeterFactoryis present (and does nothing otherwise). This will provide insight into things like how often unknown key identifiers are encountered.Part of #53654