search

jupyterhub / zero-to-jupyterhub-k8s

Helm Chart & Documentation for deploying JupyterHub on Kubernetes
https://zero-to-jupyterhub.readthedocs.io
Other
1.56k stars 799 forks source link

Add ServiceMonitor (and PodMonitor) resources instead of JUST `prometheus.io/` annotations #3458

Open frittentheke opened 4 months ago

frittentheke commented 4 months ago

Proposed change

Full disclose: This has been asked for >3 years ago in #2029 already. But I strongly believe this is a (still) valid idea and a great improvement to this powerful chart to run JupyterHub on K8s.

I'd like to propose adding ServiceMonitor (https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#monitoring.coreos.com/v1.ServiceMonitor) and potentially also PodMonitor (https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#monitoring.coreos.com/v1.PodMonitor) resources to integrate the exposed metrics with Kubernetes environments using the Prometheus Operator (https://github.com/prometheus-operator/prometheus-operator)

While I understand that you don't want to support all sorts of individual configurations, the usage of those Custom Resources to configure Prometheus is really really common.

If you look across the Kubernetes ecosystem e.g. the Prometheus-Community repo with charts for various exporters, but also also other applications meant to run on Kubernetes ...

The Prometheus Operator has also been adopted by lots of managed solutions, be it with the hyperscalers (Amazon EKS, Google Kubernetes Engine (GKE) Azure Kubernetes Service (AKS), ...) or on premise (VMware Tanzu Kubernetes Grid (TKG), OpenShift, Rancher)

Alternative options

The current set of annotations the likes of prometheus.io/scrape: true could remain in place and a new option in values.yaml could simply switch from those to dedicated ServiceMonitor and PodMonitor custom resources.

Who would use this feature?

Everybody using the Prometheus Operator to run their monitoring stack.

(Optional): Suggest a solution

see comment https://github.com/jupyterhub/zero-to-jupyterhub-k8s/issues/2029#issuecomment-1036192230 by @dmpe including a code snippet.

I gladly push a PR, if you somewhat agree that (optionally) providing Service- and PodMonitors (or active probes even ;-) ) is a good idea.

manics commented 3 months ago

I'm more inclined to support something like https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/3134 which allows arbitrary additional manifests without any expectation of support for the contents.

frittentheke commented 3 months ago

I'm more inclined to support something like #3134 which allows arbitrary additional manifests without any expectation of support for the contents.

@manics While it's certainly worth discussing if a particular functionality should come with a chart (e.g. the integration to popular monitoring stacks) or if it's a rather special case / addition which should be up to the user and individual installation to add, please kindly see me comment about the parent + subchart pattern https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/3134#issuecomment-2291614079.

I still believe that providing a ServiceMonitor with the base application chart makes sense, since all of the potential variables (ports, additional labels, ...) are already available and, as I tried to explain in my initial comment, it's quite common among other charts. To me it's kinda like adding an Ingress resource. It's just really really common and somewhat of a "batteries included" approach, even though there might be people using service meshes that would require different resource types to configure that properly.