Repository: conbench/conbench (language: python)

Issue:
Conbench on minikube with kube-prometheus (milestones 1, 2, 3)
This is supposed to implement the milestones 1, 2, 3 laid out roughly in https://github.com/conbench/conbench/issues/493#issuecomment-1413770175.

I'd like to write in more detail about what this patch is doing because code and code comments do not easily reveal the bigger picture.

This **instruments our web application with Prometheus metrics**. These metrics are exposed at the `/metrics` HTTP path of the gunicorn HTTP server that also serves the regular Flask app.

That metrics HTTP endpoint needs to be periodically pulled / "scraped" by Prometheus (in the Prometheus ecosystem, metrics are usually communicated in a pull paradigm, not a push paradigm), which then writes into its own time series database. Further below I describe how I chose to do that scraping.

I chose to use [prometheus_flask_exporter](https://github.com/rycus86/prometheus_flask_exporter) for now to get going quickly. It automatically records/measures traditional metrics around HTTP request handlers, providing insight into the health of a web application. For example: the duration required for building HTTP responses (histogram metric), the count of HTTP responses emitted with specific status codes (there's much more, this is just to give an impression).

This PR **introduces [minikube](https://minikube.sigs.k8s.io/docs/) to CI as a foundation to test k8s-specific parts of Conbench**. This allows for testing Conbench-related k8s YAML manifests in pull request CI (valuable because we embrace k8s as a/the recommended way to run Conbench, and we run it on k8s in prod ourselves). This PR introduces a CI check that after all runs Conbench on minikube and confirms that it works. 

That's really an implementation detail, but for running PostgreSQL on minikube next to Conbench I chose to use [postgres-operator](https://postgres-operator.readthedocs.io), to not spend time on configuring the stateful nature of running a DB on k8s. Great docs, this was a real time saver.

In production, on our k8s clusters running Conbench, I'd like for us to run [prometheus-operator/kube-prometheus](https://github.com/prometheus-operator/kube-prometheus) asap. Being able to roll this out with ~confidence soon is the main motivator for introducing minikube here in CI. prometheus-operator/kube-prometheus "just works", but it needs to be configured properly. The configuration surface is rather big. It's a mature and established project (boring tech, by now). It runs [Prometheus Operator](https://github.com/prometheus-operator/prometheus-operator), Grafana, Prometheus itself, [Alertmanager](https://prometheus.io/docs/alerting/latest/alertmanager/), and other proven components. All wired up for **generic k8s cluster system monitoring as well as for k8s application monitoring**. This provides a lot of value for free. I do not try to list all the value and use here. Two highlights though:
* Pre-built Grafana dashboards for per-node and per-container resource monitoring (CPU utilization etc). So far this has been a bit of a blind spot for our Conbench deployments.
* This automatically adds useful k8s metadata labels to metrics, i.e. one can for example precisely filter for the state of _one of many_ pods in a ReplicaSet. Example: `rate(flask_http_request_total{status="500", pod='conbench-deployment-79cc79bfbf-nzt54'}[2m])` shows the rate of HTTP responses with status code 500 emitted by _one specific pod_.

How does prometheus-operator/kube-prometheus know how and where to periodically scrape the /metrics HTTP endpoint served by Conbench containers? Prometheus Operator comes with custom resource definitions, one of which is the [ServiceMonitior](https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#monitoring.coreos.com/v1.ServiceMonitor). This tells Prometheus Operator to use one of its managed Prometheus instances to start scraping a custom metrics endpoint.

An exaggerated perspective is that after introducing `prometheus_flask_exporter` and  `prometheus-operator/kube-prometheus` the only code required to connect the pieces together is this:

```yaml
# This is a CRD defined by the prometheus-operator/kube-prometheus project
kind: ServiceMonitor
metadata:
  name: conbench-service-monitor
spec:
  selector:
    matchLabels:
      app: conbench
  endpoints:
    # It's important that ServiceMonitor i) refers to a Service port  and ii)
    # refers to it via its name.
    - port: conbench-service-port
      path: /metrics
      scheme: http
      # This defines the time resolution.
      interval: 30s
```

The new CI check introduced in this PR deploys all mentioned components on minikube and then confirms that
- Conbench comes up health in the k8s cluster (can interact with DB, etc)
- Conbench's `/metrics` endpoint indeed gets scraped.

Areas of friction / limitations:

* This new CI check adds quite a bit of complexity and therefore new failure paths. There is a lot of "fetch from remote" and "wait until ready", and quite a bit of coordination happening. Plenty of room for new instabilities to pop up. We will iron those out over time.
* The `/metrics` endpoint is now served by the same gunicorn HTTP server that also serves the regular web application logic. That's nice and simple initially, but we might want to change that later.
* I have for now embraced the fact that we run gunicorn with process workers -- resulting in the "multiprocess challenge" where Prometheus client metrics need to be coordinated between processes -- that coordination happens through the file system. You see `PROMETHEUS_MULTIPROC_DIR` pop up here and there in the code, that's related to this topic. In the future this complexity can disappear I hope. It's totally fine for now.
* The new CI check took ~5-7 minutes, and is therefore for now the dominating check in terms of duration. I am pretty sure it can be sped up by doing smarter coordination / concurrent deployment. The GitHub Action that I picked for running minikube itself consumes between 1 and 2 minutes. That was a bit surprising.

More work is required before we can take advantage of all this in prod. The rough plan forward from here is laid out in https://github.com/conbench/conbench/issues/493#issuecomment-1413770175, milestones 4 and onwards.

-----

Quick architecture summary in terms of information flow. This is really basic stuff, but when I learned about the Prometheus ecosystem I wish someone had explained the simple things:

### Creating metrics, writing metrics to time series DB (the write path)

- Web app code continuously updates its internal metrics state (increases counters, etc). It's always doing that.
- A scraping Prometheus instances periodically scrapes /metrics to get a _current snapshot_ (current state of all counters, gauges, etc). It knows when it does that. It persists that current state (timestamp, and all those metrics) into its internal time series database. The time resolution is only ever as big as the scrape interval allows. The scrape interval is typically 30 s or 60 s. That is, one can typically say "the mean incoming HTTP request rate in this 30 s time window was ...". That's usually sufficient!

### Reading metrics for dashboarding, alerting etc (the read path)

- Grafana has the concept of "data sources". Prometheus can be configured as a data source for Grafana. The Grafana UI can be used to explore the contents of a Prometheus time series database. It can be used to construct queries, and it can be used to build dashboards. And of course it can be used to display dashboards that were built before.

Happy to also give a tiny presentation to go through the architecture.


machine_info.py: warnings do not show up during tests
https://github.com/conbench/conbench/blob/b524d43468bc4cfce04c7aba59bc2392a595355f/conbench/machine_info.py#L100

This indirection is not of clear value:
```
class GitParseWarning(RuntimeWarning):
    pass
```

This indirection makes it so that these warnings do not show during a pytest session (and possibly never by default, I did not test this).

`RuntimeWarning` does not seem to be the right base to derive from when one wants to emit warnings from applications.

Leaving this as the _default_ https://docs.python.org/3/library/exceptions.html#UserWarning is more useful. This shows up during a pytest session:

```
conbench/tests/benchmark/test_runner.py::test_runner_simple_benchmark
  /app/conbench/machine_info.py:102: UserWarning: error in github_info(): probably not in a git repo; returning {}
    warnings.warn(
```





Discussion / hints:
Huge progress locally. `kubectl logs deployment/conbench-deployment --all-containers` showed `"GET /metrics HTTP/1.1" 200 569 "-" "Prometheus/2.41.0"` in the gunicorn access log.

That means that some of the most important wires are set up 'correctly' now, or at least in a somewhat working way that can be iterated upon. As always when having so many moving pieces this is a tiny breakthrough moment.

What's somewhat working now:
- minikube itself, postgres itself on minikube, conbench on minikube
- prometheus-flask-exporter with a special setup to retain coordination between multiple gunicorn worker processes
- A `ServiceMonitor` object (this is a custom resource, as defined by the Prometheus operator ecosystem) properly configured against the `Service` (native k8s object) that represents the Conbench k8s deployment.


Fix patch:
diff --git a/Dockerfile b/Dockerfile
index d43d62840..6a0c8b13f 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,9 +1,16 @@
-FROM python:3.11
+FROM python:3.11-slim
+
+# curl is needed for docker-compose health checks. `git` is needed by some unit
+# tests as of today.
+RUN apt-get update && apt-get install -y -q --no-install-recommends \
+    curl git && apt-get clean && rm -rf /var/lib/apt/lists/*
 
 COPY requirements-webapp.txt /tmp/
 COPY requirements-dev.txt /tmp/
-RUN pip install -r /tmp/requirements-webapp.txt
-RUN pip install -r /tmp/requirements-dev.txt
+
+RUN pip install -r /tmp/requirements-webapp.txt && \
+    pip install -r /tmp/requirements-dev.txt && \
+    rm -rf ./root/.cache
 
 # This Dockerfile currently defines the image used for production environments.
 # It also contains all test dependencies and most CI dependencies because it's
@@ -11,5 +18,26 @@ RUN pip install -r /tmp/requirements-dev.txt
 # that the `app` directory is baked in, containing current Conbench code. For
 # local development, /app may be overridden to be a volume-mount.
 WORKDIR /app
-ADD . /app
+
+# Only copy in the files that are required (instead of the entire repo root),
+# to make more use of Docker container image layer caching.
+COPY conbench /app/conbench
+COPY migrations /app/migrations
+
+# TODO: make it so that .git is not needed
+# see https://github.com/conbench/conbench/pull/667
+COPY .git /app/.git
+COPY setup.py README.md requirements-cli.txt requirements-webapp.txt requirements-dev.txt alembic.ini /app/
+
+# Inspect contents of /app
+RUN pwd && /bin/ls -1 .
+
+# Installing this package as of now is necessary not for installing
+# dependencies, but for preventing:
+# importlib.metadata.PackageNotFoundError: No package metadata was found for conbench
 RUN pip install .
+
+# Re-active this to get ideas for how the image size can be further reduced.
+#RUN echo "biggest dirs"
+#RUN cd / && du -ha . | sort -r -h | head -n 50 || true
+


Test patch:
diff --git a/conbench/tests/benchmark/test_runner.py b/conbench/tests/benchmark/test_runner.py
index 1bf3829d5..b6209beff 100644
--- a/conbench/tests/benchmark/test_runner.py
+++ b/conbench/tests/benchmark/test_runner.py
@@ -1,4 +1,5 @@
 import copy
+import os
 
 import pytest
 
@@ -13,6 +14,13 @@
     SimpleBenchmarkWithClusterInfo,
 )
 
+# this makes an implicit dependency become an explicit one, to save time
+# debugging test failures.
+assert os.path.exists(".git"), (
+    "expecting .git in current working directory, see "
+    "https://github.com/conbench/conbench/issues/668"
+)
+
 pytestmark = pytest.mark.filterwarnings("ignore::conbench.machine_info.GitParseWarning")
 
 EXAMPLE = copy.deepcopy(_fixtures.VALID_PAYLOAD)