affo commented on code in PR #2711: URL: https://github.com/apache/fluss/pull/2711#discussion_r2894494569
########## helm/README.md: ########## @@ -93,6 +93,53 @@ Important Fluss options surfaced by the chart: - internal.listener.name: Which listener is used for internal communication (defaults to INTERNAL). - tablet-server.id: Required to be unique per TabletServer. The chart auto‑derives this from the StatefulSet pod ordinal at runtime. +### Metrics and Prometheus Scraping + +The chart can enable Fluss metrics reporters and create dedicated metrics services for `coordinator` and `tablet` components. + +Example: + +```bash +helm install fluss ./fluss-helm \ + --set metrics.enabled=true +``` + +When enabled, the chart will: +- configure `metrics.reporters` from reporter names in values +- configure `metrics.reporter.<name>.<option>` entries from values +- create separate metrics services exposing the Prometheus reporter port +- optionally add metrics scrape annotations to metrics services from values +- optionally add custom labels to metrics services for `ServiceMonitor` selectors + +Values: + +| Key | Description | +| --- | --- | +| `metrics.enabled` | Enables metrics reporting and endpoint exposure. | +| `metrics.reporters` | Map of Fluss metric reporters and their options. | +| `metrics.service.portName` | Named port exposed by metrics services (usually used by ServiceMonitor endpoint `port`). | +| `metrics.service.labels` | Optional labels rendered on metrics services (usually used by ServiceMonitor selectors). | +| `metrics.annotations` | Optional map of annotations rendered on metrics services when metrics are enabled. | + +Example `values.yaml` snippet: Review Comment: Maybe better to provide the 2 prometheus examples? ########## helm/templates/svc-metrics-coordinator.yaml: ########## @@ -0,0 +1,43 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{- if .Values.metrics.enabled }} +apiVersion: v1 +kind: Service +metadata: + name: coordinator-server-metrics-hs Review Comment: ```suggestion name: {{ .Release.Name }}-coordinator-server-metrics-hs ``` wdyt? ########## website/docs/install-deploy/deploying-with-helm.md: ########## @@ -255,6 +265,40 @@ listeners: port: 9124 ``` +### Metrics and Monitoring + +When `metrics.enabled` is `true`, the Helm chart does the following: Review Comment: ```suggestion When `metrics.enabled` is `true`, adds the following `server.yaml` config entries: ``` ########## helm/templates/svc-metrics-coordinator.yaml: ########## @@ -0,0 +1,43 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{- if .Values.metrics.enabled }} +apiVersion: v1 +kind: Service +metadata: + name: coordinator-server-metrics-hs + labels: + {{- include "fluss.labels" . | nindent 4 }} + app.kubernetes.io/component: coordinator Review Comment: ```suggestion app.kubernetes.io/component: metrics ``` would help if one wants to filter all the metrics components ########## helm/values.yaml: ########## @@ -58,6 +58,16 @@ listeners: client: port: 9124 +metrics: + enabled: false + reporters: Review Comment: why don't we apply a similar pattern to the SASL PR, where one enables reporters based on a `reporters` key? In that case, we can set sane defaults for every reporter without making that appear 🤔 ########## website/docs/install-deploy/deploying-with-helm.md: ########## @@ -139,7 +139,7 @@ The Fluss Helm chart deploys the following Kubernetes resources: - **CoordinatorServer**: 1x StatefulSet with Headless Service for cluster coordination - **TabletServer**: 3x StatefulSet with Headless Service for data storage and processing - **ConfigMap**: Configuration management for `server.yaml` settings -- **Services**: Headless services providing stable pod DNS names +- **Services**: Headless services for pod communication, plus optional dedicated headless metrics services when metrics are enabled Review Comment: ```suggestion - **Services**: Headless services providing stable pod DNS names, plus optional dedicated headless services when metrics are enabled ``` ########## helm/templates/svc-metrics-coordinator.yaml: ########## @@ -0,0 +1,43 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{- if .Values.metrics.enabled }} Review Comment: This service makes sense only for the Prometheus case, not for other metrics reporters. This input: ```yaml metrics: enabled: true reporters: jmx: port: 9250-9260 prometheus: port: 9249 service: portName: metrics labels: monitoring: enabled ``` Does not make that much sense as the `service` entry is only relevant for the Prometheus case as far as we know. The service should be part of `prometheus` 🤝 ########## website/docs/install-deploy/deploying-with-helm.md: ########## @@ -255,6 +265,40 @@ listeners: port: 9124 ``` +### Metrics and Monitoring + +When `metrics.enabled` is `true`, the Helm chart does the following: + +- `metrics.reporters`: comma-separated reporter names from `metrics.reporters` +- `metrics.reporter.<name>.<option>`: one entry per reporter option in `metrics.reporters` +- Creates dedicated metrics services: + - `coordinator-server-metrics-hs` + - `tablet-server-metrics-hs` +- Exposes the Prometheus reporter port as a named service port (`metrics.service.portName`) +- Adds optional labels from `metrics.service.labels` for ServiceMonitor selection + Review Comment: Better to add the Prometheus guide here 🤝 Should explain the scraping one and the ServiceMonitor one. ########## helm/README.md: ########## @@ -93,6 +93,53 @@ Important Fluss options surfaced by the chart: - internal.listener.name: Which listener is used for internal communication (defaults to INTERNAL). - tablet-server.id: Required to be unique per TabletServer. The chart auto‑derives this from the StatefulSet pod ordinal at runtime. +### Metrics and Prometheus Scraping + +The chart can enable Fluss metrics reporters and create dedicated metrics services for `coordinator` and `tablet` components. + +Example: + +```bash +helm install fluss ./fluss-helm \ + --set metrics.enabled=true +``` + +When enabled, the chart will: +- configure `metrics.reporters` from reporter names in values +- configure `metrics.reporter.<name>.<option>` entries from values +- create separate metrics services exposing the Prometheus reporter port +- optionally add metrics scrape annotations to metrics services from values +- optionally add custom labels to metrics services for `ServiceMonitor` selectors Review Comment: Add link to Prometheus operator for reference -- 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]
