Logging updates & feedback from testing#4726
Logging updates & feedback from testing#4726vEpiphyte wants to merge 62 commits intovisi-loggingfrom
Conversation
…d AFTER reset is called are no longer being emitted by the handlers. Probably need to split the difference between a reset() method and a shutdown() method, where the shutdown method re-installs a stream handler so messages sent after shutdown are not lost to the ether.
…ue in the traceback array.
…is drained; reset, then a simple StreamHandler reinstalled. This allows for cleaner service / tool teardown as we've exited application logic at that point and may have atexit handler logic running.
… then session scope.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## visi-logging #4726 +/- ##
================================================
- Coverage 97.73% 97.67% -0.07%
================================================
Files 299 299
Lines 62492 62508 +16
================================================
- Hits 61079 61056 -23
- Misses 1413 1452 +39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vEpiphyte
added
bug
deprecation
enhancement
and removed
DO-NOT-MERGE
| logmesg = f'{logmesg} - {mesg}' | ||
|
|
||
| extra = self.core.getLogExtra(text=text) | ||
| extra = self.core.getLogExtra(text=text, user=user.iden, username=user.name) |
There was a problem hiding this comment.
If these are authenticated users, they should be automatically logged outside of the arbitrary info via the scope. We should only add them directly to extras for things like unauthenticated requests. If they don't currently get picked up, the issue is with the use of scope.
There was a problem hiding this comment.
In remote use cases, mainly driven from Optic, the link user is not going to reflect the user that is actually responsible for executing the queries. This allow us to know the who-something-was-executed-for when something went wrong; especially since we're outside of the transfer of control to the Storm user when handling these exceptions.
vEpiphyte
changed the title
2 participants
synapse.lib.loggingshutdown()API which causes the log pump to wake up, drain existing todo items, then exit. This is needed for application / command shutdown so we don't await the bg task.BaseExceptionGrouphandling toexcinfo.<none>._pumpLogStream; the last window objects in_log_winswas held onto and would not be GC'd even if its calling task was completed.Base.main()now callslogging.shutdown().synapse.lib.cmd.wrapmain()now callslogging.shutdown().extra=...definitions to use the newgetLogExtraAPIs so there would not be a regression in logged parameters.userandusernameinformation to the Storm execution errors so that information would be consistent with theStorm query executed...information.test_lib_logging.pyto increase coverage and add more focused behavioral tests.LoggerStream.expect()to raise anAssertionErrorinstead of aSynErrto avoid the possibility of a test catching aSynErrby mistake when they wanted a test assertion to fail.synapse.lib.structlog.JsonFormatterclass as deprecated.