Merge pull request #184 from alphagov/more-metrics

Improve metrics
alphagov · Jan 17, 2024 · daf817c · daf817c
2 parents 673e4fb + 68f6096
commit daf817c
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 9 deletions.
diff --git a/app/message_processors/publishing_api_message_processor.rb b/app/message_processors/publishing_api_message_processor.rb
@@ -1,12 +1,16 @@
 class PublishingApiMessageProcessor
   # Implements the callback interface required by `govuk_message_queue_consumer`
   def process(message)
+    Metrics.increment_counter(:incoming_messages)
+
     document_hash = message.payload.deep_symbolize_keys
     document = PublishingApiDocument.new(document_hash)
     document.synchronize
 
     message.ack
   rescue StandardError => e
+    Metrics.increment_counter(:message_processing_errors)
+
     # TODO: Consider options for handling errors more granularly, and for differentiating between
     # retriable (e.g. transient connection issue) and fatal (e.g. malformed document on queue)
     # errors. For now while we aren't live, log an error, send the error to Sentry, and reject the

diff --git a/app/models/publishing_api_document.rb b/app/models/publishing_api_document.rb
@@ -21,11 +21,14 @@ def initialize(
   def synchronize
     if sync?
       log("sync")
+      Metrics.increment_counter(:documents_synced)
       put_service.call(content_id, metadata, content:, payload_version:)
     elsif desync?
       log("desync (#{action_reason}))")
+      Metrics.increment_counter(:documents_desynced)
       delete_service.call(content_id, payload_version:)
     else
+      Metrics.increment_counter(:documents_skipped)
       log("skip (#{action_reason})")
     end
   end

diff --git a/app/services/discovery_engine/sync/delete.rb b/app/services/discovery_engine/sync/delete.rb
@@ -11,22 +11,24 @@ 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")
+      Metrics.increment_counter(:discovery_engine_requests, type: "delete", 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")
+      Metrics.increment_counter(
+        :discovery_engine_requests, type: "delete", status: "already_not_present"
+      )
     rescue Google::Cloud::Error => e
       log(
         Logger::Severity::ERROR,
         "Failed to delete document due to an error (#{e.message})",
         content_id:, payload_version:,
       )
       GovukError.notify(e)
-      Metrics.increment_counter(:delete_requests, status: "error")
+      Metrics.increment_counter(:discovery_engine_requests, type: "delete", status: "error")
     end
 
   private

diff --git a/app/services/discovery_engine/sync/put.rb b/app/services/discovery_engine/sync/put.rb
@@ -25,15 +25,15 @@ 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")
+      Metrics.increment_counter(:discovery_engine_requests, type: "put", status: "success")
     rescue Google::Cloud::Error => e
       log(
         Logger::Severity::ERROR,
         "Failed to add/update document due to an error (#{e.message})",
         content_id:, payload_version:,
       )
       GovukError.notify(e)
-      Metrics.increment_counter(:put_requests, status: "error")
+      Metrics.increment_counter(:discovery_engine_requests, type: "put", status: "error")
     end
 
   private

diff --git a/app/services/metrics.rb b/app/services/metrics.rb
@@ -1,14 +1,36 @@
 module Metrics
   CLIENT = PrometheusExporter::Client.default
   COUNTERS = {
+    ### User facing 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"
+    ### Synchronisation counters
+    incoming_messages: CLIENT.register(
+      :counter, "search_api_v2_incoming_messages", "number of incoming messages from Publishing API"
     ),
-    delete_requests: CLIENT.register(
-      :counter, "search_api_v2_delete_requests", "number of delete requests to Discovery Engine"
+    message_processing_errors: CLIENT.register(
+      :counter,
+      "search_api_v2_message_processing_errors",
+      "number of messages from Publishing API that failed to process",
+    ),
+    discovery_engine_requests: CLIENT.register(
+      :counter,
+      "search_api_v2_discovery_engine_requests",
+      "number of requests to Discovery Engine",
+    ),
+    documents_synced: CLIENT.register(
+      :counter, "search_api_v2_documents_synced", "number of documents synced to Discovery Engine"
+    ),
+    documents_desynced: CLIENT.register(
+      :counter,
+      "search_api_v2_documents_desynced",
+      "number of documents desynced from Discovery Engine",
+    ),
+    documents_skipped: CLIENT.register(
+      :counter,
+      "search_api_v2_documents_skipped",
+      "number of documents skipped from syncing to Discovery Engine",
     ),
   }.freeze