Merge branch '330745-eurie-add-anti-spam-latency-histogram' into 'master'

Add histogram and track latency

See merge request gitlab-org/gitlab!61582

app/services/spam/spam_verdict_service.rb

@@ -16,12 +16,17 @@ module Spam
     def execute
       spamcheck_result = nil
       spamcheck_attribs = {}
+      spamcheck_error = false
 
       external_spam_check_round_trip_time = Benchmark.realtime do
-        spamcheck_result, spamcheck_attribs = spamcheck_verdict
+        spamcheck_result, spamcheck_attribs, spamcheck_error = spamcheck_verdict
       end
 
-      # assign result to a var and log it before reassigning to nil when monitorMode is true
+      label = spamcheck_error ? 'ERROR' : spamcheck_result.to_s.upcase
+
+      histogram.observe( { result: label }, external_spam_check_round_trip_time )
+
+      # assign result to a var for logging it before reassigning to nil when monitorMode is true
       original_spamcheck_result = spamcheck_result
 
       spamcheck_result = nil if spamcheck_attribs&.fetch("monitorMode", "false") == "true"
@@ -83,8 +88,9 @@ module Spam
         end
       rescue StandardError => e
         Gitlab::ErrorTracking.log_exception(e)
+
         # Default to ALLOW if any errors occur
-        [ALLOW, attribs]
+        [ALLOW, attribs, true]
       end
     end
 
@@ -95,5 +101,9 @@ module Spam
     def logger
       @logger ||= Gitlab::AppJsonLogger.build
     end
+
+    def histogram
+      @histogram ||= Gitlab::Metrics.histogram(:gitlab_spamcheck_request_duration_seconds, 'Request duration to the anti-spam service')
+    end
   end
 end

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -129,6 +129,7 @@ The following metrics are available:
 | `pipeline_graph_links_per_job_ratio`                             | Histogram   | 13.9    | Ratio of links to job per graph                                                                                       |                                                           |
 | `gitlab_ci_pipeline_security_orchestration_policy_processing_duration_seconds` | Histogram   | 13.12    | Time in seconds it takes to process Security Policies in CI/CD pipeline                                                                    |                                                           |
 | `gitlab_ci_difference_live_vs_actual_minutes`                    | Histogram   | 13.12    | Difference between CI minute consumption counted while jobs were running (live) vs when jobs are complete (actual). Used to enforce CI minute consumption limits on long running jobs. | `plan` |
+| `gitlab_spamcheck_request_duration_seconds`                      | Histogram   | 13.12   | The duration for requests between Rails and the anti-spam engine                                                        |                                                           |
 
 ## Metrics controlled by a feature flag
 

spec/services/spam/spam_verdict_service_spec.rb

@@ -114,6 +114,33 @@ RSpec.describe Spam::SpamVerdictService do
         end
       end
     end
+
+    context 'records metrics' do
+      let(:histogram) { instance_double(Prometheus::Client::Histogram) }
+
+      using RSpec::Parameterized::TableSyntax
+
+      where(:verdict, :error, :label) do
+        Spam::SpamConstants::ALLOW             |  false |  'ALLOW'
+        Spam::SpamConstants::ALLOW             |  true  |  'ERROR'
+        Spam::SpamConstants::CONDITIONAL_ALLOW |  false |  'CONDITIONAL_ALLOW'
+        Spam::SpamConstants::BLOCK_USER        |  false |  'BLOCK'
+        Spam::SpamConstants::DISALLOW          |  false |  'DISALLOW'
+        Spam::SpamConstants::NOOP              |  false |  'NOOP'
+      end
+
+      with_them do
+        before do
+          allow(Gitlab::Metrics).to receive(:histogram).with(:gitlab_spamcheck_request_duration_seconds, anything).and_return(histogram)
+          allow(service).to receive(:spamcheck_verdict).and_return([verdict, attribs, error])
+        end
+
+        it 'records duration with labels' do
+          expect(histogram).to receive(:observe).with(a_hash_including(result: label), anything)
+          subject
+        end
+      end
+    end
   end
 
   describe '#akismet_verdict' do
@@ -313,7 +340,7 @@ RSpec.describe Spam::SpamVerdictService do
           end
 
           it 'returns nil' do
-            expect(subject).to eq([ALLOW, attribs])
+            expect(subject).to eq([ALLOW, attribs, true])
           end
         end
 
@@ -335,7 +362,7 @@ RSpec.describe Spam::SpamVerdictService do
         end
 
         it 'returns nil' do
-          expect(subject).to eq([ALLOW, attribs])
+          expect(subject).to eq([ALLOW, attribs, true])
         end
       end
     end