Helm Chart - Expose prometheus metrics to kubernetes prometheus-pod-sd #827
Conversation
…ometheus port be set and the .io/scrape annotation be set to true
CHANGELOG.md
Outdated
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| ## New features | |||
| - TBD - [#000](https://github.com/jertel/elastalert2/pull/000) - @some_elastic_contributor_tbd | |||
| - HELM CHART - [#827] - exposes prometheus metrics to kubernetes pod service discovery mechanism @PedroMSantosD | |||
There was a problem hiding this comment.
This should move down into the 4.x.x section. This top section is a template only. Also make sure the formatting matches the existing pattern. You can use [Kubernetes] instead of the HELM CHART prefix, for consistency with other changelog entries.
chart/elastalert2/README.md
Outdated
| @@ -96,3 +96,5 @@ The command removes all the Kubernetes components associated with the chart and | |||
| | `tolerations` | Tolerations for deployment | [] | | |||
| | `smtp_auth.username` | Optional SMTP mail server username. If the value is not empty, the smtp_auth secret will be created automatically. | `NULL` | | |||
| | `smtp_auth.password` | Optional SMTP mail server passwpord. This must be specified if the above field, `smtp_auth.username` is also specified. | `NULL` | | |||
| | `prometheusPort` | Optional TCP port to be used to expose prometheus metrics. if set: (1) it will pass the start parameter --prometheus_port to the command, (2) it will expose said TCP port on the POD and (3). It will add the pod annotation: prometheus.io/port: value to POD, for prometheus pod service discovery to pick the metrics | `NULL` | | |||
| | `prometheusScrapeAnnotations` | Optional Dict with the flags used by prometheus SD to know the scrape path and to keep the scrapted metrics. . Note that this values are only rendered if prometheusPort is set | prometheusScrapeAnnotations: {prometheus.io/scrape: "true" prometheus.io/path: "/"} | | |||
| @@ -48,10 +57,21 @@ spec: | |||
| command: | |||
| {{ toYaml .Values.command | indent 10 }} | |||
| {{- end }} | |||
| {{- if .Values.args }} | |||
|
|
|||
| {{- if .Values.prometheusPort }} | |||
There was a problem hiding this comment.
This looks like it can be improved to eliminate duplicate code, which will avoid future problems as the chart continues to evolve. I would expect it to look like this:
{{- if or .Values.args .Values.prometheusPort }}
args:
{{- if .Values.args }}
{{ toYaml .Values.args | indent 10 }}
{{- end }}
{{- if .Values.prometheusPort }}
{{- $enableportlist := list "--prometheus_port" (.Values.prometheusPort | toString) }}
{{ toYaml $enableportlist | indent 10 }}
{{- end }}
{{- end }}
There was a problem hiding this comment.
Fixed, thanks for a great suggestion.
That being said, I wonder if it is not better to limit the modification to allow passing metadata snnotations and spec.containers.ports configuration, and leave the user to select the args and prometheus common notation. My use case is best suited as shown on this pull request.
There was a problem hiding this comment.
Good question. We can proceed as you have it now and consider adopting a simpler approach if we need to enhance this args area in the future.
Description
This pull request allows using a value for the chart: 'prometheusPort' (Default undef), that will:
(1) expose container port prometheusPort on protocol TCP
(2) pass the argument "--prometheus_port {{prometheusPort}}" to the container, so that prometheus metrics are enabled and exposed on given metrics
(3) add pod annotations prometheus.io/port: {{prometheusPort}} AND .Values.{{prometheusScrapeAnnotations:
prometheus.io/scrape: "true"
prometheus.io/path: "/" }}
so that default prometheus service discovery for pods keeps the the metrics exposed on prometheus port.
Checklist
make test-dockerwith my changes.Questions or Comments