fix: summarize GrpcClientChannel telemetry diagnostics

[Aias00]

Summary

• Use bounded diagnostic summaries for GrpcClientChannel Settings conversion and Telemetry writes.
• Cover payload-bearing command cases without exposing message bodies in logs.

Validation

mvn -q -pl proxy -am -Dtest=GrpcClientChannelTest -DfailIfNoTests=false test

Closes #10720
Closes #10672

[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 GrpcClientChannel with a summarized version that only logs clientType, publishingTopicCount, and subscriptionCount — avoiding potential exposure of sensitive topic/resource names in logs. It also replaces full attribute string logging with just the attribute length in the parse error path.

Changes Reviewed

• proxy/src/main/java/.../GrpcClientChannel.java — added summarizeSettings() and getAttributeLength() helpers
• proxy/src/test/java/.../GrpcClientChannelTest.java — added tests for both helpers

Assessment

✅ Correctness — Logic is sound. Null checks are in place. The summary correctly uses hasPublishing()/hasSubscription() guards before accessing counts.

✅ Performance — No concerns. The summary method is lightweight and only called in error paths.

✅ Tests — Good coverage including null input, producer-only, consumer-only, and combined settings scenarios. Tests verify that sensitive resource names do not appear in the summary output.

✅ Compatibility — No public API changes. Internal helper methods are package-private.

Suggestion

The summarizeSettings() logic is duplicated across GrpcClientChannel, GrpcClientSettingsManager, and ClientActivity (see related PRs #10719, #10717). Consider extracting this into a shared utility class (e.g., SettingsLogHelper) to follow DRY principle and make future changes easier to maintain.

This can be addressed in a follow-up; the current PR is clean on its own.

Overall: Looks good. 👍

[codecov-commenter]

Codecov Report

❌ Patch coverage is 79.41176% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.52%. Comparing base (293f588) to head (a9f0ab0).

Files with missing lines	Patch %	Lines
...ketmq/proxy/grpc/v2/channel/GrpcClientChannel.java	79.41%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10721      +/-   ##
=============================================
- Coverage      48.62%   48.52%   -0.11%     
+ Complexity     13692    13667      -25     
=============================================
  Files           1381     1381              
  Lines         101464   101493      +29     
  Branches       13187    13190       +3     
=============================================
- Hits           49337    49247      -90     
- Misses         46142    46235      +93     
- Partials        5985     6011      +26     

☔ 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 object logging with compact summaries in GrpcClientChannel, preventing potentially sensitive data (topic names, group names) from appearing in logs.

Findings

• [Info] proxy/src/main/java/.../GrpcClientChannel.java — The new summarizeSettings() method is clean and well-scoped. Good use of hasPublishing()/hasSubscription() guards before accessing counts.
• [Info] proxy/src/main/java/.../GrpcClientChannel.java:110 — getAttributeLength() is a simple null-safe helper; consider using StringUtils.length(attr) from Apache Commons if already on the classpath to avoid a custom utility.
• [Info] Cross-PR duplication — The identical summarizeSettings() method appears in 3 PRs (#10721, #10719, #10717) across GrpcClientChannel, GrpcClientSettingsManager, and ClientActivity. Consider extracting a shared utility (e.g., SettingsUtils.summarize(Settings)) to avoid maintenance burden if the summary format needs to evolve.

Suggestions

• Consider a shared SettingsUtils class for the duplicated summarizeSettings() logic across the 3 related PRs.

Verdict

LGTM. Clean sanitization change with good test coverage.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Replaces raw Settings protobuf in error logs with a summarizeSettings() that logs only clientType, publishingTopicCount, and subscriptionCount. Also replaces raw attribute string with its length in the parse error path.

Findings

• [Info] GrpcClientChannel.java:91 — Error log now includes clientId for correlation and a settings summary instead of the full protobuf. Good for debugging without log noise.
• [Info] GrpcClientChannel.java:110 — Using getAttributeLength(attr) instead of the raw string in the parse error log is a good simplification.
• [Info] GrpcClientChannel.java:113-123 — summarizeSettings() is clean and null-safe.

Cross-repo Note

The same summarizeSettings() pattern appears in PRs #10719 and #10717. Consider extracting to a shared utility after all three merge.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot