[ISSUE #10728] Avoid raw POP message logs - #10729
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #10729 +/- ##
=============================================
- Coverage 48.31% 48.22% -0.09%
+ Complexity 13511 13483 -28
=============================================
Files 1380 1380
Lines 101091 101103 +12
Branches 13101 13103 +2
=============================================
- Hits 48844 48761 -83
- Misses 46285 46356 +71
- Partials 5962 5986 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
Replaces raw MessageExt toString() logging in ConsumerProcessor.filterPopResult error paths with a compact summary, preventing body/property data leakage in logs.
Findings
- [Info] Both replaced log sites (handle-null error and filterMessage failure) correctly use the summary helper instead of the raw object.
- [Info] The
summarizeMessageExtimplementation is identical to the one in PR #10731 (LocalMessageService).
Suggestions
- Code duplication: The same
summarizeMessageExtmethod appears in bothConsumerProcessor(this PR) andLocalMessageService(PR #10731). Consider extracting it into a shared utility class to keep a single source of truth. This would also apply to any future similar PRs in the series. - Test coverage is solid — verifies that body bytes and property values are excluded while key metadata is preserved.
Automated review by github-manager-bot
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
Replaces raw MessageExt.toString() in ConsumerProcessor error/warn logs with a summarizeMessageExt() summary to avoid dumping full message content.
Findings
- [Info] ConsumerProcessor.java:191,238 — Two log sites updated to use summary instead of raw object. Both are error paths where full message dumps would be noisy.
- [Info] ConsumerProcessor.java:248-259 —
summarizeMessageExt()implementation is identical to the one in PR #10731 (LocalMessageService). Consider extracting to a shared utility. - [Info] Test coverage is present.
Cross-repo Note
Same summarizeMessageExt() as PR #10731. After both merge, a shared utility in common or proxy module would reduce maintenance burden.
Automated review by github-manager-bot
Summary
Fixes #10728
Tests