1 files changed, 62 insertions, 44 deletions

diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb
index 7de3bad607a..7155017b73f 100644
--- a/app/services/spam/spam_verdict_service.rb
+++ b/app/services/spam/spam_verdict_service.rb
@@ -10,25 +10,56 @@ module Spam
       @request = request
       @user = user
       @options = options
-      @verdict_params = assemble_verdict_params(context)
+      @context = context
     end
 
     def execute
-      external_spam_check_result = external_verdict
+      spamcheck_result = nil
+      spamcheck_attribs = {}
+      spamcheck_error = false
+
+      external_spam_check_round_trip_time = Benchmark.realtime do
+        spamcheck_result, spamcheck_attribs, spamcheck_error = spamcheck_verdict
+      end
+
+      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"
+
       akismet_result = akismet_verdict
 
       # filter out anything we don't recognise, including nils.
-      valid_results = [external_spam_check_result, akismet_result].compact.select { |r| SUPPORTED_VERDICTS.key?(r) }
+      valid_results = [spamcheck_result, akismet_result].compact.select { |r| SUPPORTED_VERDICTS.key?(r) }
+
       # Treat nils - such as service unavailable - as ALLOW
       return ALLOW unless valid_results.any?
 
       # Favour the most restrictive result.
-      valid_results.min_by { |v| SUPPORTED_VERDICTS[v][:priority] }
+      final_verdict = valid_results.min_by { |v| SUPPORTED_VERDICTS[v][:priority] }
+
+      logger.info(class: self.class.name,
+                  akismet_verdict: akismet_verdict,
+                  spam_check_verdict: original_spamcheck_result,
+                  extra_attributes: spamcheck_attribs,
+                  spam_check_rtt: external_spam_check_round_trip_time.real,
+                  final_verdict: final_verdict,
+                  username: user.username,
+                  user_id: user.id,
+                  target_type: target.class.to_s,
+                  project_id: target.project_id
+                 )
+
+      final_verdict
     end
 
     private
 
-    attr_reader :user, :target, :request, :options, :verdict_params
+    attr_reader :user, :target, :request, :options, :context
 
     def akismet_verdict
       if akismet.spam?
@@ -38,54 +69,41 @@ module Spam
       end
     end
 
-    def external_verdict
+    def spamcheck_verdict
       return unless Gitlab::CurrentSettings.spam_check_endpoint_enabled
-      return if endpoint_url.blank?
 
       begin
-        result = Gitlab::HTTP.post(endpoint_url, body: verdict_params.to_json, headers: { 'Content-Type' => 'application/json' })
-        return unless result
-
-        json_result = Gitlab::Json.parse(result).with_indifferent_access
-        # @TODO metrics/logging
-        # Expecting:
-        # error: (string or nil)
-        # verdict: (string or nil)
-        # @TODO log if json_result[:error]
-
-        json_result[:verdict]
-      rescue *Gitlab::HTTP::HTTP_ERRORS => e
-        # @TODO: log error via try_post https://gitlab.com/gitlab-org/gitlab/-/issues/219223
+        result, attribs, _error = spamcheck_client.issue_spam?(spam_issue: target, user: user, context: context)
+        return [nil, attribs] unless result
+
+        # @TODO log if error is not nil https://gitlab.com/gitlab-org/gitlab/-/issues/329545
+
+        return [result, attribs] if result == NOOP || attribs["monitorMode"] == "true"
+
+        # Duplicate logic with Akismet logic in #akismet_verdict
+        if Gitlab::Recaptcha.enabled? && result != ALLOW
+          [CONDITIONAL_ALLOW, attribs]
+        else
+          [result, attribs]
+        end
+      rescue StandardError => e
         Gitlab::ErrorTracking.log_exception(e)
-        nil
-      rescue
-        # @TODO log
-        ALLOW
+
+        # Default to ALLOW if any errors occur
+        [ALLOW, attribs, true]
       end
     end
 
-    def assemble_verdict_params(context)
-      return {} unless endpoint_url.present?
-
-      project = target.try(:project)
-
-      context.merge({
-        target: {
-          title: target.spam_title,
-          description: target.spam_description,
-          type: target.class.to_s
-        },
-        user: {
-          created_at: user.created_at,
-          email: user.email,
-          username: user.username
-        },
-        user_in_project: user.authorized_project?(project)
-      })
+    def spamcheck_client
+      @spamcheck_client ||= Gitlab::Spamcheck::Client.new
+    end
+
+    def logger
+      @logger ||= Gitlab::AppJsonLogger.build
     end
 
-    def endpoint_url
-      @endpoint_url ||= Gitlab::CurrentSettings.current_application_settings.spam_check_endpoint_url
+    def histogram
+      @histogram ||= Gitlab::Metrics.histogram(:gitlab_spamcheck_request_duration_seconds, 'Request duration to the anti-spam service')
     end
   end
 end