From 2cbcab467962df8ee1bad63927c7c46b78cedd37 Mon Sep 17 00:00:00 2001
From: Alex Kalderimis <akalderimis@gitlab.com>
Date: Mon, 19 Aug 2019 23:06:21 +0000
Subject: [PATCH] Add support for sentry_extra_data in exceptions

This allows exceptions to advertise their support for sentry
and provide structured data.
---
 lib/gitlab/sentry.rb           | 17 ++++++++++++++++-
 spec/lib/gitlab/sentry_spec.rb | 29 +++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/sentry.rb b/lib/gitlab/sentry.rb
index 764db14d720..005cb3112b8 100644
--- a/lib/gitlab/sentry.rb
+++ b/lib/gitlab/sentry.rb
@@ -39,9 +39,14 @@ module Gitlab
     # development and test. If you need development and test to behave
     # just the same as production you can use this instead of
     # track_exception.
+    #
+    # If the exception implements the method `sentry_extra_data` and that method
+    # returns a Hash, then the return value of that method will be merged into
+    # `extra`. Exceptions can use this mechanism to provide structured data
+    # to sentry in addition to their message and back-trace.
     def self.track_acceptable_exception(exception, issue_url: nil, extra: {})
       if enabled?
-        extra[:issue_url] = issue_url if issue_url
+        extra = build_extra_data(exception, issue_url, extra)
         context # Make sure we've set everything we know in the context
 
         Raven.capture_exception(exception, tags: default_tags, extra: extra)
@@ -58,5 +63,15 @@ module Gitlab
         locale: I18n.locale
       }
     end
+
+    def self.build_extra_data(exception, issue_url, extra)
+      exception.try(:sentry_extra_data)&.tap do |data|
+        extra.merge!(data) if data.is_a?(Hash)
+      end
+
+      extra.merge({ issue_url: issue_url }.compact)
+    end
+
+    private_class_method :build_extra_data
   end
 end
diff --git a/spec/lib/gitlab/sentry_spec.rb b/spec/lib/gitlab/sentry_spec.rb
index af8b059b984..a48cc0d128a 100644
--- a/spec/lib/gitlab/sentry_spec.rb
+++ b/spec/lib/gitlab/sentry_spec.rb
@@ -65,6 +65,7 @@ describe Gitlab::Sentry do
 
   context '.track_acceptable_exception' do
     let(:exception) { RuntimeError.new('boom') }
+    let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1' }
 
     before do
       allow(described_class).to receive(:enabled?).and_return(true)
@@ -74,7 +75,7 @@ describe Gitlab::Sentry do
     it 'calls Raven.capture_exception' do
       expected_extras = {
         some_other_info: 'info',
-        issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1'
+        issue_url: issue_url
       }
 
       expected_tags = {
@@ -88,9 +89,33 @@ describe Gitlab::Sentry do
 
       described_class.track_acceptable_exception(
         exception,
-        issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1',
+        issue_url: issue_url,
         extra: { some_other_info: 'info' }
       )
     end
+
+    context 'the exception implements :sentry_extra_data' do
+      let(:extra_info) { { event: 'explosion', size: :massive } }
+      let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info) }
+
+      it 'includes the extra data from the exception in the tracking information' do
+        expect(Raven).to receive(:capture_exception)
+          .with(exception, a_hash_including(extra: a_hash_including(extra_info)))
+
+        described_class.track_acceptable_exception(exception)
+      end
+    end
+
+    context 'the exception implements :sentry_extra_data, which returns nil' do
+      let(:exception) { double(message: 'bang!', sentry_extra_data: nil) }
+
+      it 'just includes the other extra info' do
+        extra_info = { issue_url: issue_url }
+        expect(Raven).to receive(:capture_exception)
+          .with(exception, a_hash_including(extra: a_hash_including(extra_info)))
+
+        described_class.track_acceptable_exception(exception, extra_info)
+      end
+    end
   end
 end