[VC-34401] Add metrics settings to the Helm chart#544
Conversation
a2db6c7 to
ce73758
Compare
Signed-off-by: Richard Wall <richard.wall@venafi.com>
ce73758 to
de31f01
Compare
There was a problem hiding this comment.
Generated by make update-helm-docs.
There was a problem hiding this comment.
Naming the port is not strictly necessary, but adding it allows the PodMonitor (if enabled) to use the named port "http-metrics" rather than the port number.
There was a problem hiding this comment.
The latest thinking is that we only need to provide a PodMonitor, not a ServiceMonitor.
Other cert-manager projects also provide a ServiceMonitor, but we now consider that a legacy.
Disadvantage of ServiceMonitor is that it requires a Service, which adds unnecessary complication to the chart.
And as we understand it, with a ServiceMonitor, PrometheusOperator uses the Endpoints object created by the Service to discover the targets.
The template is copied and adapted from cert-manager:
There was a problem hiding this comment.
Thanks. I was about to ask myself "what's the recommended choice: service monitor or pod monitor", and I read your comment. Nice proactive self-reviewing!
hawksight
left a comment
There was a problem hiding this comment.
I haven't fully tested this install however I have validated all the resources look ok manually and with kubeconform:
helm template deploy/charts/venafi-kubernetes-agent --values deploy/charts/venafi-kubernetes-agent/tests/values/custom-volumes.yaml --set config.organisation="test" --set config.cluster="test" --set metrics.podmonitor.enabled=true | kubeconform -verbose -schema-location default -schema-location 'https://raw.githubusercontent.com/datreeio/CRDs-catalog/main/{{.Group}}/{{.ResourceKind}}_{{.ResourceAPIVersion}}.json'Good documentation 👍
maelvls
left a comment
There was a problem hiding this comment.
I've read the changes and manually reproduced the tests with a kind cluster. I was able to get to the same results; i've taken the liberty of fixing some of the commands (adding -i to helm upgrade, and what's the username and password for grafana, and how to import the dashboard). Thank you for the self-review comments too, it makes it such a pleasure to read.
| * Go collector: via the [default registry](https://github.com/prometheus/client_golang/blob/34e02e282dc4a3cb55ca6441b489ec182e654d59/prometheus/registry.go#L60-L63) in Prometheus client_golang. | ||
| * Process collector: via the [default registry](https://github.com/prometheus/client_golang/blob/34e02e282dc4a3cb55ca6441b489ec182e654d59/prometheus/registry.go#L60-L63) in Prometheus client_golang. | ||
| * Agent metrics: | ||
| * `data_readings_upload_size`: Data readings upload size (in bytes) sent by the jscp in-cluster agent. |
There was a problem hiding this comment.
I suppose it is "Jetstack Secure Control Plane". I copied that line from the metric description:
There was a problem hiding this comment.
Thanks. I was about to ask myself "what's the recommended choice: service monitor or pod monitor", and I read your comment. Nice proactive self-reviewing!
In #341 @tfadeyi added a metrics server to the agent.
In this PR I've made the minimum viable changes to allow that metrics server to be queried by Prometheus,
when the agent is installed by Helm in a Kubernetes cluster.
Testing
helm upgrade -i default kube-prometheus-stack \ --repo https://prometheus-community.github.io/helm-charts \ --install \ --namespace prometheus \ --create-namespace \ --values values.kube-prometheus-stack.yaml \ --waithelm upgrade venafi-kubernetes-agent ./deploy/charts/venafi-kubernetes-agent \ --install \ --create-namespace \ --namespace venafi \ --set metrics.podmonitor.enabled=trueExample Dashboards
To import the dashboard, go to http://localhost:3000/dashboards and "New" → "Import", and paste the following dashboard URL and click "Load":