Uh oh!
There was an error while loading. Please reload this page.
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.
| 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. |
TheBlueMatt
commented
Sep 16, 2024
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. |
tnull
commented
Sep 16, 2024
Hmm, good point, especially now with looping when events are replayed, this could get really spammy. Seems I merged this one prematurely. |
G8XSU
commented
Sep 16, 2024
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? |
tnull
commented
Sep 23, 2024
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.