lizhimins commented on PR #6468: URL: https://github.com/apache/rocketmq-dashboard/pull/6468#issuecomment-6192138376
Closing in favour of #6489, now merged into `rocketmq-studio` as [`a92c23144`](https://github.com/apache/rocketmq-dashboard/commit/a92c23144). Both PRs address #6465; #6489 covers strictly more alias forms, so per our rule for same-topic submissions (wider coverage wins, older lineage is only a tie-breaker) that one landed. The diagnosis is exactly right and the one-character regex change is the correct direction: the gate at `server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java:349-353` normalised with `replaceAll("\\s+", "")` while the resolver that actually builds the probe URL — `SettingsService.java:408`, `MetricsBackendType.fromProviderType(...)` at `MetricsBackendType.java:71-82` — normalises with `replaceAll("[\\s_-]+", "")`, so `victoria_metrics` was rejected at `:320-321` without a single probe being sent even though `prometheusQueryUri` would have correctly produced the tenant-prefixed `/select/0/prometheus/api/v1/query`. Your parameterised test is properly mutation-sensitive (reverting the character class turns the `victoria_metrics` / ` VICTORIA_METRICS ` / `victoria-metrics` cases red while `VictoriaMetrics` and `Victoria Metrics` stay green as controls), and `MockRestServiceServer` pins that the request really goes out, so this is not a mock-only assertion. Two problems. First, the alignment with `MetricsBackendType` is still incomplete: `MetricsBackendType.java:76` accepts `case "VICTORIAMETRICS", "VICTORIA"`, but `"victoria"` is not in `PROMETHEUS_COMPATIBLE_TYPES` at `SettingsService.java:72`, so `type="victoria"` keeps failing the connection test after this patch — and neither `"victoria"` nor `"custom"` (`MetricsBackendType.java:40`, `:81`) appears in your `@ValueSource` at `SettingsServiceTest.java:551`, which hides the gap. Second, and decisively: **#6489 fixes this same gate more completely and conflicts with this branch.** It deletes `PROMETHEUS_COMPATIBLE_TYPES` outright, replaces the check with a `resolveBackendType(String)` switch that returns a `MetricsBackendType` (covering `VICTORIA`, explicitly rejecting `CUSTOM` with a documented rationale, `default -> null` instead of the enum's silent Prometheus fallback), and passes the resolved enum into `prometheusQueryUri` so the raw string is normalised once instead of twice. That removes the duplicated allowlist — the actual root cause of the drift you found — rather than narrowing it. We are therefore going to take #6489 and close this one; the salvage path is straightforward and your analysis is not lost: if you want to keep contributing here, review #6489's `resolveBackendType` against `MetricsBackendType.fromProviderType` and send the `victoria` / `custom` alias cases plus a `MockRestServiceServer` regression as a follow-up against that shape, or propose deleting the duplicated allowlist in `MetricsBackendType`'s favour as its own change. Thanks for tracing the gate/resolver asymmetry precisely and for citing #4708 — that part of the write-up is what made this easy to confirm. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
