[ISSUE #10718] Summarize removed client settings logs

[Aias00]

Summary

• replace full gRPC Settings logging in the unused client settings cleaner with a compact summary
• log the actual consumer group name and clientId for diagnostics
• add test coverage to ensure publish/subscription resource names are not exposed by the summary

Fixes #10718

Tests

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

[Copilot]

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

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR replaces raw Settings protobuf object logging in GrpcClientSettingsManager when removing unused client settings. Instead of logging the full Settings object (which may contain sensitive topic/group names), it now logs a summary with only clientType, publishingTopicCount, and subscriptionCount.

Changes Reviewed

• proxy/src/main/java/.../GrpcClientSettingsManager.java — added summarizeClientSettings() helper, updated log statement
• proxy/src/test/java/.../GrpcClientSettingsManagerTest.java — added test verifying no resource names leak into summary

Assessment

✅ Correctness — Logic is correct. Null check is in place. The hasPublishing()/hasSubscription() guards prevent NPE on missing optional fields.

✅ Performance — No concerns. Only called in the cleanup path.

✅ Tests — Good test coverage. The test explicitly verifies that sensitive topic names ("sensitive-publish-topic", "sensitive-subscribe-topic") do not appear in the summary output, and that null input returns "null".

✅ Compatibility — No public API changes. Package-private static method.

Suggestion

Same as #10721: the summarizeClientSettings() logic here is identical to summarizeSettings() in GrpcClientChannel and ClientActivity. Consider consolidating into a shared utility to avoid maintaining 3 copies of the same logic.

Overall: Looks good. 👍

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.22%. Comparing base (00e45b8) to head (731228a).

Files with missing lines	Patch %	Lines
...roxy/grpc/v2/common/GrpcClientSettingsManager.java	50.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10719      +/-   ##
=============================================
- Coverage      48.31%   48.22%   -0.09%     
+ Complexity     13511    13481      -30     
=============================================
  Files           1380     1380              
  Lines         101091   101098       +7     
  Branches       13101    13103       +2     
=============================================
- Hits           48844    48759      -85     
- Misses         46285    46355      +70     
- Partials        5962     5984      +22     

☔ 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.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Replaces raw Settings protobuf logging with compact summaries in GrpcClientSettingsManager, and fixes a latent bug where consumerGroupInfo (the whole object) was logged instead of consumerGroup (the string name).

Findings

• [Info] proxy/src/main/java/.../GrpcClientSettingsManager.java — Good bug fix: the original consumerGroupInfo in the log was logging the full object reference, not the group name. Changing to consumerGroup is correct.
• [Info] The summarizeSettings() method is duplicated across 3 PRs (#10721, #10719, #10717). See suggestion in #10721 for a shared utility class.
• [Info] Test coverage for summarizeSettings() and getAttributeLength() is thorough, covering null, empty, and populated cases.

Suggestions

• Same as #10721: consider extracting summarizeSettings() to a shared utility.

Verdict

LGTM. Good sanitization + bonus bug fix.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Replaces raw Settings protobuf in the "remove unused grpc client settings" log with a summarizeClientSettings() summary. Also adds clientId to the log for better traceability.

Findings

• [Info] GrpcClientSettingsManager.java:285 — Adding clientId to the log is a good improvement for correlating settings removal with specific clients.
• [Info] GrpcClientSettingsManager.java:296-303 — summarizeClientSettings() implementation is identical to summarizeSettings() in PR #10721 (GrpcClientChannel). Consider consolidating.
• [Info] Test coverage validates null, empty, and populated Settings cases.

Cross-repo Note

Same summarization logic as PRs #10721 and #10717. A shared utility would prevent drift.

---

Automated review by github-manager-bot