Add labels to histogram observations

app/services/spam/spam_verdict_service.rb

@@ -16,14 +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
 
-      histogram.observe( {}, external_spam_check_round_trip_time )
+      label = spamcheck_error ? 'ERROR' : spamcheck_result.to_s.upcase
 
-      # assign result to a var and log it before reassigning to nil when monitorMode is true
+      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"
@@ -85,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
 

spec/services/spam/spam_verdict_service_spec.rb

@@ -118,13 +118,27 @@ RSpec.describe Spam::SpamVerdictService do
     context 'records metrics' do
       let(:histogram) { double('histogram') }
 
-      before do
-        expect(Gitlab::Metrics).to receive(:histogram).with(:spamcheck_latency_seconds, anything).and_return(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
 
-      it 'records latency' do
-        expect(histogram).to receive(:observe)
-        subject
+      with_them do
+        before do
+          allow(Gitlab::Metrics).to receive(:histogram).with(:spamcheck_latency_seconds, anything).and_return(histogram)
+          allow(service).to receive(:spamcheck_verdict).and_return([verdict, attribs, error])
+        end
+
+        it 'records latency with labels' do
+          expect(histogram).to receive(:observe).with(a_hash_including(result: label), anything)
+          subject
+        end
       end
     end
   end
@@ -326,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
 
@@ -348,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