[ISSUE #10724] Avoid logging full gRPC responses in ResponseWriter - #10803
[ISSUE #10724] Avoid logging full gRPC responses in ResponseWriter#10803Aias00 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Reduces risk of leaking sensitive gRPC payload data by replacing full protobuf response logging in ResponseWriter with a payload-free summary (optionally including status code), and adds tests to prevent regressions.
Changes:
- Replace debug/warn logs of full gRPC responses with
summarizeResponse(...). - Implement response summarization that includes response type and status code (when present).
- Add unit tests asserting no payload fields appear in the summary and status code is retained.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/common/ResponseWriter.java | Avoids logging full protobuf responses by logging a summary (type + optional status code). |
| proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/common/ResponseWriterTest.java | Adds coverage to ensure summaries don’t include message payload/metadata and keep status code when available. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String responseSummary = summarizeResponse(response); | ||
| log.debug("start to write response. response: {}", responseSummary); | ||
| if (isCancelled(observer)) { | ||
| log.warn("client has cancelled the request. response to write: {}", response); | ||
| log.warn("client has cancelled the request. response to write: {}", responseSummary); |
| Object status = response.getField(statusField); | ||
| if (!(status instanceof Message)) { | ||
| summary.append("{status=").append(status).append('}'); | ||
| return; | ||
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #10803 +/- ##
=============================================
- Coverage 48.31% 48.23% -0.09%
+ Complexity 13521 13494 -27
=============================================
Files 1380 1380
Lines 101104 101123 +19
Branches 13107 13111 +4
=============================================
- Hits 48853 48779 -74
- Misses 46287 46357 +70
- Partials 5964 5987 +23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Tests
Closes #10724