Add logging around event processing. #3314
Conversation
Currently we don't have any visibility if BackgroundProcessor takes considerably more of time to process events, adding logs to help debug such issues.
G8XSU
force-pushed
the
event-logging
branch
from
6104129
to
85eb814
Compare
| @@ -325,9 +325,17 @@ macro_rules! define_run_body { | |||
| let mut have_decayed_scorer = false; | |||
|
|
|||
| loop { | |||
| log_trace!($logger, "Processing ChannelManager events..."); | |||
There was a problem hiding this comment.
Lmk, if we want to combine all of them into one "Processing events..."
Or want to log individual events as well or just types of events.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3314 +/- ##
==========================================
+ Coverage 89.66% 89.98% +0.32%
==========================================
Files 126 126
Lines 102411 104389 +1978
Branches 102411 104389 +1978
==========================================
+ Hits 91826 93935 +2109
+ Misses 7857 7742 -115
+ Partials 2728 2712 -16 ☔ View full report in Codecov by Sentry. |
|
This is gonna be really verbose, and I don't think we want that. Instead, we should probably log before actually calling into the event handlers (with the event type) in the event generating methods. |
Hmm, good point, especially now with looping when events are replayed, this could get really spammy. Seems I merged this one prematurely. |
|
Uh oh.. |
Mhh, yeah, that improves things, but I'm not convinced this really solves the verbosity issue as in case of event handling failure we immediately loop back without any added delay. That means if we hit a persistence failure we might keep spamming the logs until the failure is resolved (e.g., we reconnected to remote persistence). Also, in case logs are persisted towards the same backend, we might further bog down IO with spamming a lot log writes? I think if we add logging there, we need to add some delay (exponential backoff?), as we had considered before but didn't end up doing in the fallible event handler PR(s)? Btw, if we're now logging at the event-handling level, we could also log event handling errors? |
|
Now tracking here to make sure we don't forget: #3331 |
Currently we don't have any visibility if BackgroundProcessor takes considerably more of time to process events,
adding logs to help debug such issues.