Add event ids to all log calls #110
Conversation
|
|
||
| private static Action<ILogger, Guid, Exception> _foundRevocationOfKey; | ||
|
|
||
| private static Action<ILogger, XElement, Exception> _ExceptionWhileProcessingRevocationElement; |
There was a problem hiding this comment.
Oh no, field starts with capital E 😄
|
Looks good |
assuming the CI passes
|
🆙 📅 |
| public static void LogDebugF(this ILogger logger, FormattableString message) | ||
| public static void UsingFallbackKeyWithExpirationAsDefaultKey(this ILogger logger, Guid keyId, DateTimeOffset expirationDate) | ||
| { | ||
| _usingFallbackKeyWithExpirationAsDefaultKey(logger, keyId, expirationDate, null); |
There was a problem hiding this comment.
Named param for the null at the end of these?
There was a problem hiding this comment.
Nope, its Action<?> so param name would be argN
There was a problem hiding this comment.
Oh right, I keep forgetting that.
There was a problem hiding this comment.
good ol' inline comment /* parameterX */ null
|
Just small comments, then |
pakrym
force-pushed
the
pakrym/log-eventid-3
branch
from
b05c495 to
c48173c
Compare
|
You're going to have to rerun perf tests against this library. The code was originally written in such a way that gathering the data to be written to the log was deferred until it was known for certain that there was a logger listening. For instance, the expensive /cc @blowdart |
|
@GrabYourPitchforks I left checks in that places (https://github.com/aspnet/DataProtection/pull/110/files#diff-f79751618d4c52e43acbad2bc4c474b6R237) |
|
@pakrym great :) |
#94
@victorhurdugaci @moozzyk @anpete