-
-
Notifications
You must be signed in to change notification settings - Fork 207
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
Logging loop with n log sentry #1824
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.
Thank you for fixing this Simon!
i work around this with an asynclocal. can anyone think of a better approach? |
Yes. We can use NLog's built-in context for this. See my update, and LMK if you agree. Thanks. |
@mattjohnsonpint much nicer. thanks |
Think it is a bad idea to "abuse" the NLog ScopeContext to protect against bad calling pattern. The NLog ScopeContext is for the user to capture application-state, not for secret infrastructure problems. Just like you hopefully won't "abuse" the Serilog LogContext for injecting special context values. If you really insist on abusing the NLog NestedDiagnosticsLogicalContext, then you should render the Sentry-LogEvent outside this secret-scope. As layouts depending on ${ndlc} will have side-effects from this usage. But I recommend that you keep the special infrastructure problems secret, and use a local AsyncLocal-member-variable. |
@snakefoot that's a good point. @SimonCropp - let's go back to the async local approach you had. Thanks. |
done #1830 |
although this is not the exact scenario as #1741 as #1701, it is the same root cause. essentially when inside logging we have anything that causes a log event it results in an infinite loop. in #1701
NLog.SentryTarget.Write
occursScope.Evaluate
occursAspNetCore.SentryMiddleware
which is failingNLog.SentryTarget.Write