Annotate Pods for Prometheus Metrics Collection

[teodor-yanev]

#1097

Overview:

Prometheus relies on annotations to discover which pods to scrape metrics from. This issue covers the addition of the necessary annotations to the relevant pods (for Mediator), enabling Prometheus to dynamically and accurately discover and scrape them.

Goals:

1. Annotate the mediator pods to expose the metrics endpoint to Prometheus.
2. Ensure that the annotations are correctly picked up by Prometheus for scraping.
3. Maintain a standard scrape configuration that can be easily scaled and managed across different pods/services in the future.

Acceptance Criteria:

1. After deployment, the mediator pods should have the appropriate prometheus.io annotations.
2. Prometheus should start scraping metrics from the mediator pods based on the annotations.
3. There should be visible metric data in Prometheus (and AMP, when integrated) related to the mediator pods.

[JAORMX]

Can you add more description to this PR and the commit itself? it's good to have this for historical purposes and context.

[JAORMX]

Let's make these annotations configurable as follows:

      annotations:
        prometheus.io/scrape: "true"
        prometheus.io/port: "9090"
        prometheus.io/path: '/metrics'
        {{- with .Values.podAnnotations }}
        {{- toYaml . | nindent 8 }}
        {{- end }}

This way, operators may set extra annotations via podAnnotations in values.yaml

you should then set podAnnotations in values.yaml as {}

[teodor-yanev]

That was a great point, thanks. Done now.

[teodor-yanev]

Can you add more description to this PR and the commit itself? it's good to have this for historical purposes and context.

Done (copied the issue description).

[JAORMX]

Why is the single quote needed? I wouldn't expect wrapping here and it seems to me like it would break the YAML.

[teodor-yanev]

The addition of the single quotes was a measure to ensure compatibility with 'ko', which tries to parse the raw Helm template as YAML before Helm processes it. Without the single quotes, 'ko' was throwing a parsing error.

The single quotes ensure that, from a YAML perspective, the Helm directives are treated as a simple string. When Helm processes this template, it replaces the single quotes and the inner directives with the intended values, resulting in valid, rendered YAML.
This serves as a safeguard against parsing issues with tools that try to read the raw Helm template. Once Helm processes the template, the resulting YAML is as expected without any single quotes.

I'm open to other suggestions 😄.

[teodor-yanev]

make helm                                   
cd deployment/helm; rm -f templates/combined.yml && \
            ko resolve --platform=linux/amd64,linux/arm64 --base-import-paths --push=false -f templates/ > templates/combined.yml && \
                helm dependency update && \
                helm package --version="0.1.0" .
2023/10/04 15:25:25 Using base cgr.dev/chainguard/static:latest@sha256:ef5add7fd46cf1ce7d33d6de517833ac5c7e749db9b15249f9c472a772f3af27 for github.com/stacklok/mediator/cmd/server
2023/10/04 15:25:26 Building github.com/stacklok/mediator/cmd/server for linux/amd64
2023/10/04 15:25:26 Unexpected error running "go build": context canceled
2023/10/04 15:25:26 Building github.com/stacklok/mediator/cmd/server for linux/arm64
2023/10/04 15:25:26 Unexpected error running "go build": context canceled
Error: error processing import paths in "templates/deployment.yaml": yaml: line 31: did not find expected node content
make: *** [helm] Error 1

[JAORMX]

maaan! that's weird. @evankanderson is this expected? I would have thought that ko would use the rendered template instead.

[evankanderson]

The ordering here is backwards -- we run ko before helm templating happens, because we want to do:

1. Build container and replace package references in-line
2. Package helm templates into an OCI image
3. Replace helm parameters by ArgoCD on the cluster.

I'm surprised that helm is removing the single quotes (and not replacing them with the equivalent, like : | on the preceeding line). I'd expected:

....
    annotations:
      'prometheus.io/scrape: "true"
      prometheus.io/port: "9090"
      prometheus.io/path: "/metrics"'

What you want to do is the trick I did with labels above, and hide the template inside a comment on a single line.

I like having the resolved image associated with the helm chart so they are locked together, but I'm not thrilled with ko's insistence on valid YAML. I'd be open to another tool which did the build and substitution if there's an obvious one available.

BTW, for testing, what I've been doing is make helm build and then helm template mediator ./deployment/helm/mediator-*.tgz ...

[evankanderson]

Note that helm template is entirely valid as a checking mechanism, because Argo is using that to get the set of objects to monitor in the cluster, and not actually running helm install or helm upgrade.

[teodor-yanev]

Thank you both!

With the current setup, the yaml I get:

kind: Deployment
metadata:
  name: mediator
  labels:
    # This includes a newline, so ko sees this as valid yaml
    app.kubernetes.io/instance: mediator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: mediator
    app.kubernetes.io/version: "2023-07-31"
    helm.sh/chart: mediator-0.1.0
spec:
  # We'll use autoscaling, sometimes clamped to one instance
  selector:
    matchLabels:
      app: 'mediator'
  template:
    metadata:
      labels:
        app: 'mediator'
      annotations:
        # This includes a newline, so ko sees this as valid yaml
        prometheus.io/scrape: "true"
        prometheus.io/port: "9090"
        prometheus.io/path: "/metrics"
    spec:
      serviceAccountName: mediator
      containers:
...

[evankanderson]

Testing this with make helm && helm template mediator ./deployment/helm/mediator-0.1.0.tgz | grep -A 23 '^kind: Deployment', I get:

kind: Deployment
metadata:
  name: mediator
  labels:
  # This includes a newline, so ko sees this as valid yaml
  # 
    app.kubernetes.io/instance: mediator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: mediator
    app.kubernetes.io/version: "2023-07-31"
    helm.sh/chart: mediator-0.1.0
spec:
  # We'll use autoscaling, sometimes clamped to one instance
  selector:
    matchLabels:
      app: 'mediator'
  template:
    metadata:
      labels:
        app: 'mediator'
      annotations: '
        prometheus.io/path: /metrics
        prometheus.io/port: "9090"
        prometheus.io/scrape: "true"'

Try the comment trick and see if that helps.

[evankanderson]

Nit, but I'm not going to block submit: This is probably "pod annotations" or the like. We probably don't want these for other objects, like Services or Ingresses.

Moved attributes to be configurable.