Skip to content

Commit

Permalink
Add initial metrics support (attempt 2)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
csutter committed Jan 16, 2024
1 parent a6f55a9 commit e768a54
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 2 deletions.
15 changes: 13 additions & 2 deletions app/services/discovery_engine/query/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions app/services/discovery_engine/sync/delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ 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,
"Failed to delete document due to an error (#{e.message})",
content_id:, payload_version:,
)
GovukError.notify(e)
Metrics.increment_counter(:delete_requests, status: "error")
end

private
Expand Down
2 changes: 2 additions & 0 deletions app/services/discovery_engine/sync/put.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +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")
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")
end

private
Expand Down
22 changes: 22 additions & 0 deletions app/services/metrics.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e768a54

Please sign in to comment.