From e768a548786177da7affda3e1cf1e8a50f328464 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Tue, 16 Jan 2024 10:17:50 +0000 Subject: [PATCH] Add initial metrics support (attempt 2) The missing PrometheusExporter on workers has been resolved (hopefully), so we should be able to go ahead with this. - Add `Metrics` module to abstract metric updates using default `PrometheusExporter` client - Add basic initial metrics for number of search/put/delete requests and failures --- app/services/discovery_engine/query/search.rb | 15 +++++++++++-- app/services/discovery_engine/sync/delete.rb | 3 +++ app/services/discovery_engine/sync/put.rb | 2 ++ app/services/metrics.rb | 22 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 app/services/metrics.rb diff --git a/app/services/discovery_engine/query/search.rb b/app/services/discovery_engine/query/search.rb index ef51a37..8759c75 100644 --- a/app/services/discovery_engine/query/search.rb +++ b/app/services/discovery_engine/query/search.rb @@ -29,7 +29,14 @@ def result_set attr_reader :query_params, :client def response - @response ||= client.search(discovery_engine_params).response + @response ||= client.search(discovery_engine_params).response.tap do + Metrics.increment_counter( + :search_requests, + query_present: query.present?, + filter_present: filter.present?, + best_bets_applied: best_bets_boost_specs.present?, + ) + end end def discovery_engine_params @@ -87,11 +94,15 @@ def boost_spec { condition_boost_specs: [ *NewsRecencyBoost.new.boost_specs, - *BestBetsBoost.new(query).boost_specs, + *best_bets_boost_specs, ], } end + def best_bets_boost_specs + @best_bets_boost_specs ||= BestBetsBoost.new(query).boost_specs + end + def suggested_queries # TODO: Highlighting isn't actually supported by Discovery Engine, and this _always_ returns a # single suggestion, but we need to do this for API compatibility with Finder Frontend. diff --git a/app/services/discovery_engine/sync/delete.rb b/app/services/discovery_engine/sync/delete.rb index 74ef6b8..d8a95de 100644 --- a/app/services/discovery_engine/sync/delete.rb +++ b/app/services/discovery_engine/sync/delete.rb @@ -11,12 +11,14 @@ def call(content_id, payload_version: nil) client.delete_document(name: document_name(content_id)) log(Logger::Severity::INFO, "Successfully deleted", content_id:, payload_version:) + Metrics.increment_counter(:delete_requests, status: "success") rescue Google::Cloud::NotFoundError => e log( Logger::Severity::INFO, "Did not delete document as it doesn't exist remotely (#{e.message}).", content_id:, payload_version:, ) + Metrics.increment_counter(:delete_requests, status: "already_not_present") rescue Google::Cloud::Error => e log( Logger::Severity::ERROR, @@ -24,6 +26,7 @@ def call(content_id, payload_version: nil) content_id:, payload_version:, ) GovukError.notify(e) + Metrics.increment_counter(:delete_requests, status: "error") end private diff --git a/app/services/discovery_engine/sync/put.rb b/app/services/discovery_engine/sync/put.rb index 3e8a26d..c7aed45 100644 --- a/app/services/discovery_engine/sync/put.rb +++ b/app/services/discovery_engine/sync/put.rb @@ -25,6 +25,7 @@ def call(content_id, metadata, content: "", payload_version: nil) ) log(Logger::Severity::INFO, "Successfully added/updated", content_id:, payload_version:) + Metrics.increment_counter(:put_requests, status: "success") rescue Google::Cloud::Error => e log( Logger::Severity::ERROR, @@ -32,6 +33,7 @@ def call(content_id, metadata, content: "", payload_version: nil) content_id:, payload_version:, ) GovukError.notify(e) + Metrics.increment_counter(:put_requests, status: "error") end private diff --git a/app/services/metrics.rb b/app/services/metrics.rb new file mode 100644 index 0000000..cf425dd --- /dev/null +++ b/app/services/metrics.rb @@ -0,0 +1,22 @@ +module Metrics + CLIENT = PrometheusExporter::Client.default + COUNTERS = { + search_requests: CLIENT.register( + :counter, "search_api_v2_search_requests", "number of incoming search requests" + ), + put_requests: CLIENT.register( + :counter, "search_api_v2_put_requests", "number of put requests to Discovery Engine" + ), + delete_requests: CLIENT.register( + :counter, "search_api_v2_put_requests", "number of delete requests to Discovery Engine" + ), + }.freeze + + def self.increment_counter(counter, labels = {}) + Rails.logger.warn("Unknown counter: #{counter}") and return unless COUNTERS.key?(counter) + + COUNTERS[counter].observe(1, labels) + rescue StandardError + # Metrics are best effort only, don't raise if they fail + end +end