From 184990d5946aab7491fc227c503352eda42183ac Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Thu, 1 Aug 2024 15:42:09 +0100 Subject: [PATCH 1/3] Push fact check metrics to prometheus Inspired by https://github.com/alphagov/search-api-v2/pull/209 This adds the prometheus-client gem, and then uses it to send fact check emails metrics to prometheus via the pushgateway. We can't use the conventional prometheus collector, because this code runs as a script, rather than a web process. This should not be merged until we've set the PROMETHEUS_PUSHGATEWAY_URL environment variable. --- Gemfile | 1 + Gemfile.lock | 3 +++ app/lib/fact_check_email_handler.rb | 5 +++-- script/mail_fetcher | 11 ++++++++++- test/unit/lib/fact_check_email_handler_test.rb | 15 ++++++++++----- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index cf012ca12..5fbfa574f 100644 --- a/Gemfile +++ b/Gemfile @@ -31,6 +31,7 @@ gem "mousetrap-rails" gem "nested_form", git: "https://github.com/alphagov/nested_form.git", branch: "add-wrapper-class" gem "null_logger" gem "plek" +gem "prometheus-client" gem "rails_autolink" gem "rest-client", require: false gem "sassc-rails" diff --git a/Gemfile.lock b/Gemfile.lock index 5920fb7e0..540d5e1b2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -594,6 +594,8 @@ GEM ast (~> 2.4.1) racc plek (5.2.0) + prometheus-client (4.2.3) + base64 prometheus_exporter (2.1.1) webrick pry (0.14.1) @@ -850,6 +852,7 @@ DEPENDENCIES nested_form! null_logger plek + prometheus-client pry-byebug rack rails (= 7.0.8.3) diff --git a/app/lib/fact_check_email_handler.rb b/app/lib/fact_check_email_handler.rb index 63bf3c2a9..04987bc16 100644 --- a/app/lib/fact_check_email_handler.rb +++ b/app/lib/fact_check_email_handler.rb @@ -9,8 +9,9 @@ class FactCheckEmailHandler attr_accessor :fact_check_config - def initialize(fact_check_config) + def initialize(fact_check_config, unprocessed_emails_gauge) @fact_check_config = fact_check_config + @unprocessed_emails_gauge = unprocessed_emails_gauge end def process_message(message) @@ -35,7 +36,7 @@ def process unprocessed_emails_count += 1 end - GovukStatsd.gauge("unprocessed_emails.count", unprocessed_emails_count) + @unprocessed_emails_gauge.set(unprocessed_emails_count) rescue StandardError => e # Occasionally, there is an error when connecting to the mailbox in production. # It seems a very transient error, and since the job is run every few minutes isn't really a problem, but if the diff --git a/script/mail_fetcher b/script/mail_fetcher index 728c16255..bf4ffe099 100755 --- a/script/mail_fetcher +++ b/script/mail_fetcher @@ -20,7 +20,12 @@ end Rails.logger.info "Running MailFetcher in #{Rails.env} mode - #{Time.zone.now.utc}" -handler = FactCheckEmailHandler.new(Publisher::Application.fact_check_config) +registry = Prometheus::Client.registry +gauge = registry.gauge( + :publisher_fact_check_unprocessed_emails_total, + docstring: "Number of unprocessed fact check emails", +) +handler = FactCheckEmailHandler.new(Publisher::Application.fact_check_config, gauge) # The lock is created and belongs to this process for as long as the `life`. # When the block has finished executing, the lock is explicitly released. @@ -50,3 +55,7 @@ rescue StandardError => e end Rails.logger.info "Finished running MailFetcher in #{Rails.env} mode - #{Time.zone.now.utc}" +Prometheus::Client::Push.new( + job: "publisher_fact_check_emails", + gateway: ENV.fetch("PROMETHEUS_PUSHGATEWAY_URL"), +).add(registry) diff --git a/test/unit/lib/fact_check_email_handler_test.rb b/test/unit/lib/fact_check_email_handler_test.rb index 8a104db5d..621586c4e 100644 --- a/test/unit/lib/fact_check_email_handler_test.rb +++ b/test/unit/lib/fact_check_email_handler_test.rb @@ -1,29 +1,34 @@ require "test_helper" class FactCheckEmailHandlerTest < ActiveSupport::TestCase + setup do + @gauge = stub + end + def handler - FactCheckEmailHandler.new(Publisher::Application.fact_check_config) + FactCheckEmailHandler.new(Publisher::Application.fact_check_config, @gauge) end test "#process ignores 'out of office' emails" do out_of_office_message = Mail.new { subject("Automatic reply: out of office") } Mail.stubs(:all).yields(out_of_office_message) + @gauge.expects(:set).with(0) handler.process assert out_of_office_message.is_marked_for_delete? end - test "#process sends count of unprocessed emails to Graphite" do + test "#process sends count of unprocessed emails to Prometheus" do processed_message = Mail.new { subject("Automatic reply: out of office") } unprocessed_message = Mail.new { subject("Any other subject") } - GovukStatsd.expects(:gauge).with("unprocessed_emails.count", 1) + @gauge.expects(:set).with(1) Mail.stubs(:all).multiple_yields(processed_message, unprocessed_message) handler.process end - test "#process does not send count of unprocessed emails to Graphite when emails cannot be retrieved" do + test "#process does not send count of unprocessed emails to Prometheus when emails cannot be retrieved" do Mail.stubs(:all).raises(StandardError) - GovukStatsd.expects(:gauge).never + @gauge.expects(:set).never handler.process end From 0a76c36a2248c75d30576a15fc60019f7dfc691e Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Thu, 1 Aug 2024 16:42:07 +0100 Subject: [PATCH 2/3] Stub prometheus gauge in tests --- test/integration/fact_check_email_test.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/integration/fact_check_email_test.rb b/test/integration/fact_check_email_test.rb index dd39cff0c..94763603c 100644 --- a/test/integration/fact_check_email_test.rb +++ b/test/integration/fact_check_email_test.rb @@ -27,7 +27,7 @@ def assert_correct_state(key, value, state) message[key] = value Mail.stubs(:all).yields(message) - FactCheckEmailHandler.new(fact_check_config).process + FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)).process answer.reload assert answer.public_send("#{state}?") @@ -41,7 +41,7 @@ def assert_correct_state(key, value, state) answer.destroy! - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process assert message.is_marked_for_delete? @@ -53,7 +53,7 @@ def assert_correct_state(key, value, state) message = fact_check_mail_for(answer) Mail.stubs(:all).yields(message) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process answer.reload @@ -71,7 +71,7 @@ def assert_correct_state(key, value, state) Mail.stubs(:all).yields(fact_check_mail_for(answer)) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process answer.reload @@ -96,7 +96,7 @@ def assert_correct_state(key, value, state) fact_check_mail_for(answer1, body: "Third Message"), ) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process answer1.reload @@ -122,7 +122,7 @@ def assert_correct_state(key, value, state) Mail.stubs(:all).yields(message) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process assert_not message.is_marked_for_delete? @@ -139,7 +139,7 @@ def assert_correct_state(key, value, state) Mail.stubs(:all).multiple_yields(message_cc, message_bcc) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process assert message_cc.is_marked_for_delete? @@ -152,7 +152,7 @@ def assert_correct_state(key, value, state) Mail.stubs(:all).yields(message) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process_message(message) assert message.is_marked_for_delete? end @@ -163,7 +163,7 @@ def assert_correct_state(key, value, state) Mail.stubs(:all).yields(message) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) handler.process_message(message) assert message.is_marked_for_delete? @@ -182,7 +182,7 @@ def assert_correct_state(key, value, state) fact_check_mail_for(answer2, body: "Second Message"), ) - handler = FactCheckEmailHandler.new(fact_check_config) + handler = FactCheckEmailHandler.new(fact_check_config, stub.stubs(:set)) invocations = 0 handler.process do From f40059d5a7eea2bfa433fd496879b16c1089fd02 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 2 Aug 2024 11:08:22 +0100 Subject: [PATCH 3/3] Update job name to be more general The job name should really be the same for all metrics published by this _thing_. We currently have publisher (for the web app) and publisher-worker for the worker. I think publisher-metrics is probably a good job name for these specificly metricsy things. --- script/mail_fetcher | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/mail_fetcher b/script/mail_fetcher index bf4ffe099..de67d0c32 100755 --- a/script/mail_fetcher +++ b/script/mail_fetcher @@ -56,6 +56,6 @@ end Rails.logger.info "Finished running MailFetcher in #{Rails.env} mode - #{Time.zone.now.utc}" Prometheus::Client::Push.new( - job: "publisher_fact_check_emails", + job: "publisher-metrics", gateway: ENV.fetch("PROMETHEUS_PUSHGATEWAY_URL"), ).add(registry)