Remove redundant channel ID logging #4217
Conversation
|
👋 Thanks for assigning @valentinewallace as a reviewer! |
joostjager
force-pushed
the
chan-logging
branch
2 times, most recently
from
5bae991 to
bf104c0
Compare
lightning/src/util/test_utils.rs
Outdated
| let s = format!("{:<55} {}", context, record.args); | ||
| let chan_id = record.channel_id.map(|id| format!("{}", id)); | ||
| let chan_str = chan_id.as_deref().and_then(|s| s.get(..6)).unwrap_or(""); | ||
| let s = format!("{:<55} {:<6} {}", context, chan_str, record.args); |
There was a problem hiding this comment.
Rather than just updating the test logger, let's instead maybe add a Display implementation for Record that includes all the relevant information? After we do that we'll need to update the one in fuzzing and IMO the docs on the logger to make clear that we intend to not include redundant information in the human-readable part of the log record (and that they absolutely need to include it if they want useful logs).
There was a problem hiding this comment.
Display added and docs updated.
joostjager
force-pushed
the
chan-logging
branch
from
bf104c0 to
9491fa7
Compare
As a preparatory step for unifying test log output.
Makes sure that 4 and 5 character log levels line up.
Now that we have Display implemented on Record, the various test logger implementations can make use of that.
joostjager
force-pushed
the
chan-logging
branch
from
9491fa7 to
98cffe3
Compare
Previously, log messages often included the channel ID both in the message text and in the structured `channel_id` field. This led to redundant information and made it difficult to quickly identify which log lines pertain to a given channel, as the ID could appear in different positions or sometimes not at all. This change removes the channel ID from the message in all cases where it is already present in the structured field. A test run was used to verify that the structured field always appeared in these log calls. Some exceptions remain—for example, calls where the structured field contains a temporary channel ID and the message contains the final ID were left unchanged.
joostjager
force-pushed
the
chan-logging
branch
2 times, most recently
from
44dec3f to
2f6b50a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4217 +/- ##
=======================================
Coverage 89.31% 89.32%
=======================================
Files 180 180
Lines 138055 138129 +74
Branches 138055 138129 +74
=======================================
+ Hits 123309 123377 +68
+ Misses 12142 12139 -3
- Partials 2604 2613 +9
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:
|
Since most log messages no longer include the channel ID in their text, this commit updates the test logger to output the structured channel ID instead. The ID is truncated for readability.
joostjager
force-pushed
the
chan-logging
branch
from
2f6b50a to
f2483eb
Compare
3 participants
This PR improves channel ID logging consistency and clarity.
Removes redundant channel ID text from log messages where the ID is
already included as a structured field. This reduces duplication and makes
it easier to filter logs by channel.
Updates the test logger to display the structured channel ID instead of
relying on message text. The logged ID is truncated for readability.
Together, these changes standardize how channel IDs appear in logs and make
them easier to parse and analyze.
Example output: