Skip to content

[ISSUE #10710] Sanitize lite offline Settings failure logs - #10712

Open
Aias00 wants to merge 1 commit into
apache:developfrom
Aias00:fix/lite-offline-settings-log-summary
Open

[ISSUE #10710] Sanitize lite offline Settings failure logs#10712
Aias00 wants to merge 1 commit into
apache:developfrom
Aias00:fix/lite-offline-settings-log-summary

Conversation

@Aias00

@Aias00 Aias00 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace full protobuf Settings logging in lite offline cleanup failures with a compact diagnostic summary
  • keep client type, group, first topic, and subscription count for troubleshooting
  • add a regression test that ensures subscription filter details are not leaked

Fixes #10710

Tests

  • JAVA_HOME=$(/usr/libexec/java_home -v 1.8) mvn -pl proxy -Dtest=GrpcClientSettingsManagerTest -DfailIfNoTests=false test

Copilot AI review requested due to automatic review settings July 31, 2026 06:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by github-manager-bot

Summary

Replace full protobuf Settings logging in lite offline cleanup failures with a compact diagnostic summary to prevent sensitive subscription filter details from leaking into logs.

Findings

  • [Info] GrpcClientSettingsManager.java:257-282 — summarizeLiteSettings() correctly extracts only safe diagnostic fields (clientType, group, first topic, subscription count) without exposing subscription filter expressions.
  • [Info] GrpcClientSettingsManager.java:260 — Null check for settings is good defensive coding.
  • [Info] GrpcClientSettingsManagerTest.java — Regression test verifies that filter details are not leaked in the summary output. Good coverage.
  • [Info] — Fixes #10710, clean and focused change.

Suggestions

  • Minor: summarizeLiteSettings only shows the first topic (subscriptions(0)). If multiple topics are involved in a failure, only the first is visible. This is acceptable for error-path logging, but consider logging subscriptionCount prominently so operators know there may be more topics involved.
  • Consider making the method package-private instead of protected static if no subclass override is intended — this limits the API surface.

Overall

Clean fix for a log sanitization issue. Good test coverage. LGTM.


Automated review by github-manager-bot

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.24%. Comparing base (00e45b8) to head (4a5403a).

Files with missing lines Patch % Lines
...roxy/grpc/v2/common/GrpcClientSettingsManager.java 57.14% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10712      +/-   ##
=============================================
- Coverage      48.31%   48.24%   -0.08%     
+ Complexity     13511    13482      -29     
=============================================
  Files           1380     1380              
  Lines         101091   101104      +13     
  Branches       13101    13103       +2     
=============================================
- Hits           48844    48773      -71     
- Misses         46285    46344      +59     
- Partials        5962     5987      +25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Lite client offline cleanup logs full gRPC Settings on failure

4 participants