[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
Closes #10683

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.51%. Comparing base (293f588) to head (d62d31c).

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.60%   48.51%   -0.10%     
+ Complexity     13690    13659      -31     
=============================================
  Files           1381     1381              
  Lines         101464   101483      +19     
  Branches       13187    13191       +4     
=============================================
- Hits           49318    49233      -85     
- Misses         46158    46236      +78     
- Partials        5988     6014      +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]

LGTM. Code changes look good.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot

[Aias00]

Rebased onto current develop and revalidated GrpcClientSettingsManagerTest. This PR is now the canonical metric collector address validation fix; #10684 was superseded and closed because this branch additionally covers whitespace and port-range validation.