Skip to content

Commit

Permalink
Get controller labels from controller, not params
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
richardTowers committed Oct 3, 2023
1 parent 4cc1ecc commit 096f2fc
Showing 1 changed file with 40 additions and 2 deletions.
42 changes: 40 additions & 2 deletions lib/govuk_app_config/govuk_prometheus_exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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."
Expand Down

0 comments on commit 096f2fc

Please sign in to comment.