[ISSUE #10788] Tolerate invalid metric collector address

[Aias00]

What is changed

• Validate metricCollectorAddress before building gRPC client metric settings in metricCollectorMode=on.
• Disable client metric collection for that settings response when the configured address is blank, malformed, non-numeric, or outside the valid port range.
• Add coverage for valid and invalid collector addresses.

Fixes #10788

Verification

• mvn -pl proxy -Dtest=GrpcClientSettingsManagerTest test

[Copilot]

Pull request overview

Hardens Proxy gRPC client metric settings generation so an invalid optional metricCollectorAddress no longer breaks settings merge when metricCollectorMode=on, aligning behavior with Issue #10788.

Changes:

• Add validation/parsing for metricCollectorAddress and disable client metric collection when the address is blank/malformed/invalid.
• Log warnings when metric collection is disabled due to invalid address input.
• Add unit tests covering valid and invalid collector addresses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/common/GrpcClientSettingsManager.java	Adds safe parsing/validation for metric collector address and disables metrics on invalid input.
proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/common/GrpcClientSettingsManagerTest.java	Adds tests validating correct behavior for valid/invalid metric collector addresses.

Suppressed comments (1)

proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/common/GrpcClientSettingsManagerTest.java:104

• The new parsing logic in GrpcClientSettingsManager handles multiple invalid forms (blank, missing host/port, non-numeric port, out-of-range port), but this test only covers the missing-port case. Expanding it to cover the other invalid branches will better prevent regressions for the hardening introduced in this PR (and also restore the previous config instead of forcing defaults).

        ConfigurationManager.getProxyConfig().setMetricCollectorMode(MetricCollectorMode.ON.getModeString());
        ConfigurationManager.getProxyConfig().setMetricCollectorAddress("localhost");
        try {
            Settings settings = this.grpcClientSettingsManager.mergeMetric(Settings.getDefaultInstance());

            assertEquals(false, settings.getMetric().getOn());

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Extracts metric collector address parsing into a dedicated method with comprehensive validation (blank check, format check, port range check), gracefully disabling metric collection on invalid input instead of throwing.

Findings

• [Info] GrpcClientSettingsManager.java:155 — split(":", -1) correctly handles trailing colons (e.g. "host:" → ["host", ""]), which is good defensive practice.
• [Info] GrpcClientSettingsManager.java:163 — Port range validation (1–65535) is correct and prevents invalid endpoint construction.
• [Info] Tests cover both valid and invalid address scenarios, which is good.

Suggestions

• Minor: consider logging the original config value at DEBUG level rather than WARN for the blank case, since a blank default is a common initial state. Not blocking.

LGTM — clean defensive improvement with good test coverage.

---

Automated review by github-manager-bot

[codecov-commenter]

Codecov Report

❌ Patch coverage is 53.84615% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.30%. Comparing base (eddb235) to head (f7dddf8).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...roxy/grpc/v2/common/GrpcClientSettingsManager.java	53.84%	9 Missing and 3 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10789      +/-   ##
=============================================
- Coverage      48.34%   48.30%   -0.05%     
+ Complexity     13527    13516      -11     
=============================================
  Files           1380     1380              
  Lines         101104   101157      +53     
  Branches       13107    13124      +17     
=============================================
- Hits           48882    48863      -19     
- Misses         46267    46310      +43     
- Partials        5955     5984      +29     

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