affo commented on code in PR #2711:
URL: https://github.com/apache/fluss/pull/2711#discussion_r2882783642
##########
helm/README.md:
##########
@@ -93,6 +93,48 @@ 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 add scrape annotations to
Services.
+
+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
+- optionally add metrics scrape annotations to Fluss Services from values
+
+Values:
+
+| Key | Description |
+| --- | --- |
+| `metrics.enabled` | Enables metrics reporting and endpoint exposure. |
+| `metrics.reporters` | Map of Fluss metric reporters and their options. |
+| `metrics.annotations` | Optional map of annotations rendered on Fluss
Services when metrics are enabled. |
+
+Example `values.yaml` snippet:
+
+```yaml
+metrics:
+ enabled: true
+ reporters:
+ grph:
+ port: 9020
+ host: example-localhost
+ protocol: TCP
+ interval: "60 SECONDS"
+ prometheus:
+ port: 9249
+ annotations:
Review Comment:
Ah sorry, I did not notice that!
I think you are right, often times, exposing metrics through a headless
service is the default way to go for solving connectivity issues and let
Prometheus use the k8s DNS, hence:
- your design is correct, `annotations` makes sense under `metrics`
- but, you should expose the port on the service if metrics are enabled,
otherwise this won't work!
Basically, we have to look at what a user would do:
__Case 1: no Prometheus operator__
Use annotations-based discovery.
You want to add to your service:
```yaml
metadata:
annotations:
prometheus.io/scrape: "true"
prometheus.io/port: "9249"
prometheus.io/path: "/metrics"
```
The user would configure its Prometheus server to scrape those with relabel
rules.
__Case 2: WITH Prometheus operator__
No annotation is required, but you need your service to expose the metrics
port with a name and make it selectable via labels! E.g.:
```yaml
...
metadata:
labels:
monitoring: enabled
spec:
clusterIP: None # headless: DNS returns pod IPs
selector: {}
ports:
- name: metrics # IMPORTANT: ServiceMonitor references
this name
port: 9100
targetPort: 9100
```
You would use the ServiceMonitor CRD like this (maintained by the user):
```yaml
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
labels:
release: prometheus # IMPORTANT: must match Prometheus' CR
serviceMonitorSelector
spec:
selector:
matchLabels:
monitoring: enabled # matches the Service labels above
namespaceSelector:
matchNames: []
endpoints:
- port: metrics # matches Service port name
path: /metrics
interval: 30s
scrapeTimeout: 10s
```
__Conclusion__
We should allow:
- specific service `annotations` for covering case 1
- add a named port for case 2 -> ideally the user may choose the port name
- add the possibility to add labels to the service for selection for case 2
Probably better to have a separate metrics service in order to have a clear
separation of concerns 🤝
--
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]
