fix: harden local POP scans and diagnostics - #10731
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #10731 +/- ##
=============================================
- Coverage 48.31% 48.22% -0.10%
+ Complexity 13511 13479 -32
=============================================
Files 1380 1380
Lines 101091 101102 +11
Branches 13101 13103 +2
=============================================
- Hits 48844 48752 -92
- Misses 46285 46363 +78
- Partials 5962 5987 +25 ☔ 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 LocalMessageService.popMessage with a compact summary that omits body bytes and property values, reducing log noise and preventing accidental data exposure.
Findings
- [Info]
summarizeMessageExtis well-structured with proper null checks formessageExt,body, andproperties. UsingTreeSetfor property keys ensures deterministic output — good for log parsing. - [Info] The
bodySizefallback to0whengetBody() == nullis correct and consistent.
Suggestions
- Cross-PR note: PR #10729 introduces an identical
summarizeMessageExtmethod inConsumerProcessor. Consider extracting this into a shared utility class (e.g.,proxy/utils/MessageExtSummary.java) to avoid duplication and simplify future maintenance. - Minor: the log message prefix
"Queue offset [{}] of msg is strange, not equal to the stored in msg, msgSummary:{}"could be slightly more readable with a space after the colon before the summary.
Tests
Test testSummarizeMessageExtDoesNotExposeBodyOrPropertyValues correctly asserts that body content and property values are absent from the output. Good coverage.
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 log output with a concise summarizeMessageExt() that logs only key metadata (topic, msgId, queueId, queueOffset, commitLogOffset, bodySize, propertyKeys) without exposing message body content.
Findings
- [Info] LocalMessageService.java:292 — Good change: avoids dumping full message content in warn logs. The summary includes enough context for debugging (offsets, msgId) without leaking body data.
- [Info] LocalMessageService.java:317-328 —
summarizeMessageExt()implementation is clean. Null-safe, usesTreeSetfor deterministic key ordering. - [Info] Test coverage validates null input and normal cases.
Cross-repo Note
The same summarizeMessageExt() method appears in PR #10729 (ConsumerProcessor). Consider extracting to a shared utility class (e.g., MessageExtUtil) to avoid duplication once both PRs are merged.
Automated review by github-manager-bot
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Defensive fix with proper validation and test coverage. LGTM.
Automated review by github-manager-bot
3a172f6 to
3711596
Compare
Summary
Validation
mvn -q -pl proxy -am -Dtest=LocalMessageServiceTest -DfailIfNoTests=false testCloses #10730
Closes #10786