Merge branch 'sh-add-api-exception-to-logs' into 'master'

Include exception and backtrace in API logs

Closes #35362

See merge request gitlab-org/gitlab!19671

changelogs/unreleased/sh-add-api-exception-to-logs.yml

+---
+title: Include exception and backtrace in API logs
+merge_request: 19671
+author:
+type: other

lib/api/api.rb

@@ -21,6 +21,7 @@ module API
                     Gitlab::GrapeLogging::Loggers::ClientEnvLogger.new,
                     Gitlab::GrapeLogging::Loggers::RouteLogger.new,
                     Gitlab::GrapeLogging::Loggers::UserLogger.new,
+                    Gitlab::GrapeLogging::Loggers::ExceptionLogger.new,
                     Gitlab::GrapeLogging::Loggers::QueueDurationLogger.new,
                     Gitlab::GrapeLogging::Loggers::PerfLogger.new,
                     Gitlab::GrapeLogging::Loggers::CorrelationIdLogger.new

lib/api/helpers.rb

@@ -9,6 +9,7 @@ module API
     GITLAB_SHARED_SECRET_HEADER = "Gitlab-Shared-Secret"
     SUDO_PARAM = :sudo
     API_USER_ENV = 'gitlab.api.user'
+    API_EXCEPTION_ENV = 'gitlab.api.exception'
 
     def declared_params(options = {})
       options = { include_parent_namespaces: false }.merge(options)
@@ -387,6 +388,9 @@ module API
         Gitlab::Sentry.track_acceptable_exception(exception, extra: params)
       end
 
+      # This is used with GrapeLogging::Loggers::ExceptionLogger
+      env[API_EXCEPTION_ENV] = exception
+
       # lifted from https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L60
       trace = exception.backtrace
 

lib/gitlab/grape_logging/loggers/exception_logger.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GrapeLogging
+    module Loggers
+      class ExceptionLogger < ::GrapeLogging::Loggers::Base
+        def parameters(request, _)
+          # grape-logging attempts to pass the logger the exception
+          # (https://github.com/aserafin/grape_logging/blob/v1.7.0/lib/grape_logging/middleware/request_logger.rb#L63),
+          # but it appears that the rescue_all in api.rb takes
+          # precedence so the logger never sees it. We need to
+          # store and retrieve the exception from the environment.
+          exception = request.env[::API::Helpers::API_EXCEPTION_ENV]
+
+          return {} unless exception.is_a?(Exception)
+
+          data = {
+            exception: {
+              class: exception.class.to_s,
+              message: exception.message
+            }
+          }
+
+          if exception.backtrace
+            data[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(exception.backtrace)
+          end
+
+          data
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb

+require 'spec_helper'
+
+describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do
+  subject { described_class.new }
+
+  let(:mock_request) { OpenStruct.new(env: {}) }
+
+  describe ".parameters" do
+    describe 'when no exception is available' do
+      it 'returns an empty hash' do
+        expect(subject.parameters(mock_request, nil)).to eq({})
+      end
+    end
+
+    describe 'when an exception is available' do
+      let(:exception) { RuntimeError.new('This is a test') }
+      let(:mock_request) do
+        OpenStruct.new(
+          env: {
+            ::API::Helpers::API_EXCEPTION_ENV => exception
+          }
+        )
+      end
+
+      let(:expected) do
+        {
+          exception: {
+            class: 'RuntimeError',
+            message: 'This is a test'
+          }
+        }
+      end
+
+      it 'returns the correct fields' do
+        expect(subject.parameters(mock_request, nil)).to eq(expected)
+      end
+
+      context 'with backtrace' do
+        before do
+          current_backtrace = caller
+          allow(exception).to receive(:backtrace).and_return(current_backtrace)
+          expected[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(current_backtrace)
+        end
+
+        it 'includes the backtrace' do
+          expect(subject.parameters(mock_request, nil)).to eq(expected)
+        end
+      end
+    end
+  end
+end