From 096f2fc067da13f4f9060a244ecde14214b4b344 Mon Sep 17 00:00:00 2001
From: Richard Towers <richard.towers@digital.cabinet-office.gov.uk>
Date: Mon, 2 Oct 2023 14:25:04 +0100
Subject: [PATCH] Get controller labels from controller, not params

I've got an open PR to do this for the default default in
prometheus_exporter itself:

github[.]com/discourse/prometheus_exporter/pull/293

But it's unclear whether the maintainers will accept that, and if so how
long that will take.

In the meantime, we've got quite a bit of noise in our metrics because
it's currently possible for an HTTP request to overwrite the action /
controller labels by passing them in as request parameters like this:

    curl -v http://127.0.0.1:3000/ --data 'controller=test'

This commit extends the default middleware so we have more sensible
defaults for both Rails and Sinatra apps. I couldn't work out any good
labels for sinatra apps that wouldn't be potentially high cardinality,
so those are just blank.
---
 .../govuk_prometheus_exporter.rb              | 42 ++++++++++++++++++-
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/lib/govuk_app_config/govuk_prometheus_exporter.rb b/lib/govuk_app_config/govuk_prometheus_exporter.rb
index 986b2f1..ac55301 100644
--- a/lib/govuk_app_config/govuk_prometheus_exporter.rb
+++ b/lib/govuk_app_config/govuk_prometheus_exporter.rb
@@ -4,6 +4,44 @@
 require "prometheus_exporter/middleware"
 
 module GovukPrometheusExporter
+
+  #
+  # See https://github.com/discourse/prometheus_exporter/pull/293
+  #
+  # RailsMiddleware can be removed and replaced with the default middleware if
+  # that PR is merged / released
+  #
+  class RailsMiddleware < PrometheusExporter::Middleware
+    def default_labels(env, result)
+      controller_instance = env["action_controller.instance"]
+      action = controller = nil
+      if controller_instance
+        action = controller_instance.action_name
+        controller = controller_instance.controller_name
+      elsif (cors = env["rack.cors"]) && cors.respond_to?(:preflight?) && cors.preflight?
+        # if the Rack CORS Middleware identifies the request as a preflight request,
+        # the stack doesn't get to the point where controllers/actions are defined
+        action = "preflight"
+        controller = "preflight"
+      end
+      {
+        action: action || "other",
+        controller: controller || "other"
+      }
+    end
+  end
+
+  class SinatraMiddleware < PrometheusExporter::Middleware
+    def default_labels(env, result)
+      # The default prometheus exporter middleware uses the controller and
+      # action as labels.  These aren't meaningful in Sinatra applications, and
+      # other options (such as request.path_info) have potentially very high
+      # cardinality.  For now, just accept that we can't be more specific than
+      # the application / pod and don't provide any other labels
+      {}
+    end
+  end
+
   def self.should_configure
     # Allow us to force the Prometheus Exporter for persistent Rake tasks...
     if ENV["GOVUK_PROMETHEUS_EXPORTER"] == "force"
@@ -50,11 +88,11 @@ def self.configure(collectors: [], default_aggregation: PrometheusExporter::Metr
       server.start
 
       if defined?(Rails)
-        Rails.application.middleware.unshift PrometheusExporter::Middleware, instrument: :prepend
+        Rails.application.middleware.unshift RailsMiddleware, instrument: :prepend
       end
 
       if defined?(Sinatra)
-        Sinatra.use PrometheusExporter::Middleware
+        Sinatra.use ::GovukPrometheusExporter::SinatraMiddleware
       end
     rescue Errno::EADDRINUSE
       warn "Could not start Prometheus metrics server as address already in use."