-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SLVS-1722 Log extra properties of SLCore log events #5924
SLVS-1722 Log extra properties of SLCore log events #5924
Conversation
public string threadName; | ||
public string loggerName; | ||
// nullable | ||
public string stackTrace; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are missing the loggedAt
property. Even if we don't use it for now, I think we should add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a comment
@@ -34,25 +34,26 @@ public class LoggerListener : ILoggerListener | |||
[ImportingConstructor] | |||
public LoggerListener(ILogger logger) | |||
{ | |||
this.logger = logger; | |||
this.logger = logger.ForContext(SLCoreStrings.SLCoreName, SLCoreStrings.SLCoreInternalLogContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's enough to add just one context that is a string of type "SlCore Logs". It would occupies less space than [SLCore > Logs].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave it as it was before, just SLCore
break; | ||
} | ||
|
||
if (parameters.stackTrace != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the null check inside the LogVerbose method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't really want to move this logic inside the logger. It's not really the logger's business IMO
d01d7dd
to
dc2b4c0
Compare
77eab71
to
201c413
Compare
201c413
to
ad18a41
Compare
Quality Gate passedIssues Measures |
7e16bea
into
feature/hardening-q4
SLVS-1722