Refactor logging for efficiency, add fifo, context printing in text mode #4133
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4133 +/- ##
==========================================
- Coverage 97.35% 97.29% -0.06%
==========================================
Files 261 261
Lines 59821 59954 +133
==========================================
+ Hits 58237 58331 +94
- Misses 1584 1623 +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:
|
invisig0th
changed the title
invisig0th
changed the title
|
|
||
| if rcode is False: | ||
| self.clear() | ||
| self.set_status(500) | ||
| self.sendRestErr('StormRuntimeError', f'Extended HTTP API {iden} never set status code.') | ||
| self.sendRestErr('StormRuntimeError', 'Extended HTTP API never set status code.') |
There was a problem hiding this comment.
Why is the iden being removed here?
There was a problem hiding this comment.
It seemed like an unnecessary disclosure of "internals" to a caller who knows what API endpoint they hit. Happy to discuss. 👍
|
|
||
| logtodo = [] | ||
| logbase = None | ||
| loglock = asyncio.Lock() |
There was a problem hiding this comment.
These structures ( loglock / logevnt )will bind to the currently running loop when they are first locked / waited on; meaning we'll end up with isolation issues if we move to isolated ioloops for testing.
| s_logging.logbase = await s_base.Base.anit() | ||
| s_logging.logbase._fini_at_exit = True | ||
|
|
||
| s_logging.logbase.schedCoro(s_logging._feedLogTask()) |
There was a problem hiding this comment.
The logbase feels like something we should be able to refcount & teardown on cell fini to prevent the feedtask from running beyond the lifetime of a cell; similar to how we handle other global tasks.
There was a problem hiding this comment.
There's a bit of a "not necessarily via a single cell in one python interpreter" issue here that would need to be discussed, but def worth talking through 👍
| @@ -6626,7 +6647,9 @@ async def enableCronJob(self, iden): | |||
| ''' | |||
| await self.agenda.enable(iden) | |||
| await self.feedBeholder('cron:enable', {'iden': iden}, gates=[iden]) | |||
| logger.info(f'Enabled cron job {iden}', extra=await self.getLogExtra(iden=iden, status='MODIFY')) | |||
There was a problem hiding this comment.
We need to retain these status keys.
No description provided.