kta1kri commented on PR #12513: URL: https://github.com/apache/seatunnel/pull/12513#issuecomment-5868023374
Thanks @DanielLeens and @davidzollo for the careful review. **Issue 1 (metrics on 5801) — addressed by repointing Prometheus to REST API v2.** You're right that disabling `rest-api` closes the member-port (5801) HTTP layer, so `/hazelcast/rest/instance/metrics` there stops answering. The same samples are already served on the REST API v2 / Jetty listener (8080), so there's no need to keep the unauthenticated v1 REST layer alive just for telemetry: - `RestHttpGetCommandProcessor.handleMetrics` (5801) and `MetricsServlet.doGet` (8080) both write `nodeExtension.getMetricFamilySamples()` via `TextFormat.writeFormat`, so the output is identical. - `JettyService` registers `MetricsServlet` at `/metrics` and `/openmetrics` unconditionally, with the default context-path `""`, and the chart already runs the Jetty listener (`enable-http: true`, port 8080). So I repointed the default Prometheus pod annotations (master + worker) from `5801` `/hazelcast/rest/instance/metrics` to `8080` `/metrics`, with a comment explaining why. v1 stays fully off, and telemetry keeps working. Caveat worth noting: if REST v2 Basic auth is enabled the scrape needs credentials — it's off by default in the chart. I traced this in source rather than on a live cluster; on a running install `curl http://<pod>:8080/metrics` should return the same Prometheus text that `5801/hazelcast/rest/instance/metrics` used to. **Issue 2 (upgrade/migration note) — added.** - `docs/en/introduction/concepts/incompatible-changes.md`: new entry under `## dev` (affected component, impact, migration to REST v2 on 8080, re-enabling v1 via a custom ConfigMap plus a `NetworkPolicy` on 5801 if truly required, and the metrics repoint). - `docs/en/getting-started/kubernetes/helm.md` and `docs/zh/getting-started/kubernetes/helm.md`: a matching note, including that `helm upgrade` updates the ConfigMap but running pods keep the old config until restarted (mounted via `subPath`, no checksum annotation). **CI:** enabled Actions in my fork and pushed the above as a new commit, so the workflows should run now. Happy to adjust any of the wording. -- 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]
