-
Notifications
You must be signed in to change notification settings - Fork 177
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
Fix(Logger): CNX-8443 speckle connectors overwrite static logger instance #3142
Fix(Logger): CNX-8443 speckle connectors overwrite static logger instance #3142
Conversation
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.
Flagging as requesting changes... but it's more to prevent accidental merging while we discuss these points.
@oguzhankoral it seems you've introduce an infinite loop in your |
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.
From several places in our code, We're pushing scoped logs via the LogContext.PushProperty
(see Operations.Receive
) for an example.
Discussed briefly with Alan and Oguzhan, we should find an alternative for these to use SpeckleLog
explicitly
…ctors-Overwrite-Static-Logger-Instance # Conflicts: # Core/Tests/Speckle.Core.Tests.Unit/Speckle.Core.Tests.Unit.csproj
Pulled from |
Previously we set out logger instance onto static
Log.Logger
and properties onto staticGlobalLogContext
. Now we set global properties onLoggerConfiguration
instance before creating instance fromILogger
.Update: We only set
id
property after creatingILogger
. It was causing infinite loop on the case of having no account.