-
Notifications
You must be signed in to change notification settings - Fork 36
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
Link logger with gin context. #76
Link logger with gin context. #76
Conversation
807fb1a
to
fdbfb8b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #76 +/- ##
==========================================
+ Coverage 94.24% 94.36% +0.12%
==========================================
Files 2 2
Lines 139 142 +3
==========================================
+ Hits 131 134 +3
Misses 5 5
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@cedric-appdirect with these changes we are seeing again the issues mentioned in #69 as this is mutatin the logger. Not only that, but this is wrong in the sense that is not grabbing the correct logger (using Not sure what is the right fix here as these changes try to save a mutated logger, so we either save the original logger (maybe useless?) or we save the event? |
@cedric-appdirect Created #97 to fix this. Let me know your thoughts. |
Indeed, you are right missed that during the rebase. |
This is trowing just an idea at the moment. This would have to be adjusted once #69 is landed. We might want to make it even optional via config as mutating the logger has performance impact.