Disable release detection when SDK is not configured to send events (#…

…1471)

* Move release detection test out of configuration specs

* Detect release "after" SDK config is fully initialized

This allows the SDK to log release detection messages with the logger
user provides.

* Don't detect release unless the SDK is ready to send events

This makes sure the users don't see release detection warnings when they
don't even plan to use it.

Closes #885

sentry-ruby/lib/sentry-ruby.rb

@@ -84,6 +84,7 @@ def registered_patches
     def init(&block)
       config = Configuration.new
       yield(config) if block_given?
+      config.detect_release
       apply_patches(config)
       client = Client.new(config)
       scope = Scope.new(max_breadcrumbs: config.max_breadcrumbs)

sentry-ruby/lib/sentry/configuration.rb

@@ -198,7 +198,6 @@ def initialize
       self.project_root = Dir.pwd
       self.propagate_traces = true
 
-      self.release = detect_release
       self.sample_rate = 1.0
       self.send_modules = true
       self.send_default_pii = false
@@ -314,17 +313,19 @@ def stacktrace_builder
       )
     end
 
-    private
-
     def detect_release
-      detect_release_from_env ||
+      return unless sending_allowed?
+
+      self.release ||= detect_release_from_env ||
         detect_release_from_git ||
         detect_release_from_capistrano ||
         detect_release_from_heroku
     rescue => e
       log_error("Error detecting release", e, debug: debug)
     end
 
+    private
+
     def excluded_exception?(incoming_exception)
       excluded_exception_classes.any? do |excluded_exception|
         matches_exception?(excluded_exception, incoming_exception)

sentry-ruby/spec/sentry/configuration_spec.rb

@@ -154,141 +154,6 @@
     end
   end
 
-  context 'being initialized without a release' do
-    let(:fake_root) { "/tmp/sentry/" }
-
-    before do
-      allow(File).to receive(:directory?).and_return(false)
-      allow_any_instance_of(described_class).to receive(:project_root).and_return(fake_root)
-    end
-
-    it 'defaults to nil' do
-      expect(subject.release).to eq(nil)
-    end
-
-    it 'uses `SENTRY_RELEASE` env variable' do
-      ENV['SENTRY_RELEASE'] = 'v1'
-
-      expect(subject.release).to eq('v1')
-
-      ENV.delete('SENTRY_CURRENT_ENV')
-    end
-
-    context "when git is available" do
-      before do
-        allow(File).to receive(:directory?).and_return(false)
-        allow(File).to receive(:directory?).with(".git").and_return(true)
-      end
-      it 'gets release from git' do
-        allow(Sentry).to receive(:`).with("git rev-parse --short HEAD 2>&1").and_return("COMMIT_SHA")
-
-        expect(subject.release).to eq('COMMIT_SHA')
-      end
-    end
-
-    context "when Capistrano is available" do
-      let(:revision) { "2019010101000" }
-
-      before do
-        Dir.mkdir(fake_root) unless Dir.exist?(fake_root)
-        File.write(filename, file_content)
-      end
-
-      after do
-        File.delete(filename)
-        Dir.delete(fake_root)
-      end
-
-      context "when the REVISION file is present" do
-        let(:filename) do
-          File.join(fake_root, "REVISION")
-        end
-        let(:file_content) { revision }
-
-        it "gets release from the REVISION file" do
-          expect(subject.release).to eq(revision)
-        end
-      end
-
-      context "when the revisions.log file is present" do
-        let(:filename) do
-          File.join(fake_root, "..", "revisions.log")
-        end
-        let(:file_content) do
-          "Branch master (at COMMIT_SHA) deployed as release #{revision} by alice"
-        end
-
-        it "gets release from the REVISION file" do
-          expect(subject.release).to eq(revision)
-        end
-      end
-    end
-
-    context "when running on heroku" do
-      before do
-        allow(File).to receive(:directory?).and_return(false)
-        allow(File).to receive(:directory?).with("/etc/heroku").and_return(true)
-      end
-
-      context "when it's on heroku ci" do
-        it "returns nil" do
-          begin
-            original_ci_val = ENV["CI"]
-            ENV["CI"] = "true"
-
-            expect(subject.release).to eq(nil)
-          ensure
-            ENV["CI"] = original_ci_val
-          end
-        end
-      end
-
-      context "when it's not on heroku ci" do
-        around do |example|
-          begin
-            original_ci_val = ENV["CI"]
-            ENV["CI"] = nil
-
-            example.run
-          ensure
-            ENV["CI"] = original_ci_val
-          end
-        end
-
-        it "returns nil + logs an warning if HEROKU_SLUG_COMMIT is not set" do
-          logger = double("logger")
-          expect(::Sentry::Logger).to receive(:new).and_return(logger)
-          expect(logger).to receive(:warn).with(Sentry::LOGGER_PROGNAME) { described_class::HEROKU_DYNO_METADATA_MESSAGE }
-
-          expect(described_class.new.release).to eq(nil)
-        end
-
-        it "returns HEROKU_SLUG_COMMIT" do
-          begin
-            ENV["HEROKU_SLUG_COMMIT"] = "REVISION"
-
-            expect(subject.release).to eq("REVISION")
-          ensure
-            ENV["HEROKU_SLUG_COMMIT"] = nil
-          end
-        end
-      end
-
-      context "when having an error detecting the release" do
-        it "logs the error" do
-          string_io = StringIO.new
-          logger = Logger.new(string_io)
-          allow_any_instance_of(described_class).to receive(:logger).and_return(logger)
-          allow_any_instance_of(described_class).to receive(:detect_release_from_git).and_raise(TypeError.new)
-
-          subject
-
-          expect(string_io.string).to include("ERROR -- sentry: Error detecting release: TypeError")
-        end
-      end
-    end
-  end
-
   describe "config: backtrace_cleanup_callback" do
     it "defaults to nil" do
       expect(subject.backtrace_cleanup_callback).to eq(nil)

sentry-ruby/spec/sentry_spec.rb

@@ -464,4 +464,192 @@
       expect(described_class.get_current_scope.user).to eq(id: 1)
     end
   end
+
+  describe 'release detection' do
+    let(:fake_root) { "/tmp/sentry/" }
+
+    before do
+      allow(File).to receive(:directory?).and_return(false)
+      allow_any_instance_of(Sentry::Configuration).to receive(:project_root).and_return(fake_root)
+      ENV["SENTRY_DSN"] = DUMMY_DSN
+    end
+
+    subject { described_class.configuration }
+
+    it 'defaults to nil' do
+      described_class.init
+      expect(described_class.configuration.release).to eq(nil)
+    end
+
+    it "respects user's config" do
+      described_class.init do |config|
+        config.release = "foo"
+      end
+
+      expect(described_class.configuration.release).to eq("foo")
+    end
+
+    it 'uses `SENTRY_RELEASE` env variable' do
+      ENV['SENTRY_RELEASE'] = 'v1'
+
+      described_class.init
+      expect(described_class.configuration.release).to eq('v1')
+
+      ENV.delete('SENTRY_CURRENT_ENV')
+    end
+
+    context "when the DSN is not set" do
+      before do
+        ENV.delete("SENTRY_DSN")
+      end
+
+      it "doesn't detect release" do
+        ENV['SENTRY_RELEASE'] = 'v1'
+
+        described_class.init
+        expect(described_class.configuration.release).to eq(nil)
+
+        ENV.delete('SENTRY_CURRENT_ENV')
+      end
+    end
+
+    context "when the SDK is not enabled under the current env" do
+      it "doesn't detect release" do
+        ENV['SENTRY_RELEASE'] = 'v1'
+
+        described_class.init do |config|
+          config.enabled_environments = "production"
+        end
+
+        expect(described_class.configuration.release).to eq(nil)
+
+        ENV.delete('SENTRY_CURRENT_ENV')
+      end
+    end
+
+    context "when git is available" do
+      before do
+        allow(File).to receive(:directory?).and_return(false)
+        allow(File).to receive(:directory?).with(".git").and_return(true)
+      end
+      it 'gets release from git' do
+        allow(Sentry).to receive(:`).with("git rev-parse --short HEAD 2>&1").and_return("COMMIT_SHA")
+
+        described_class.init
+        expect(described_class.configuration.release).to eq('COMMIT_SHA')
+      end
+    end
+
+    context "when Capistrano is available" do
+      let(:revision) { "2019010101000" }
+
+      before do
+        Dir.mkdir(fake_root) unless Dir.exist?(fake_root)
+        File.write(filename, file_content)
+      end
+
+      after do
+        File.delete(filename)
+        Dir.delete(fake_root)
+      end
+
+      context "when the REVISION file is present" do
+        let(:filename) do
+          File.join(fake_root, "REVISION")
+        end
+        let(:file_content) { revision }
+
+        it "gets release from the REVISION file" do
+          described_class.init
+          expect(described_class.configuration.release).to eq(revision)
+        end
+      end
+
+      context "when the revisions.log file is present" do
+        let(:filename) do
+          File.join(fake_root, "..", "revisions.log")
+        end
+        let(:file_content) do
+          "Branch master (at COMMIT_SHA) deployed as release #{revision} by alice"
+        end
+
+        it "gets release from the REVISION file" do
+          described_class.init
+          expect(described_class.configuration.release).to eq(revision)
+        end
+      end
+    end
+
+    context "when running on heroku" do
+      before do
+        allow(File).to receive(:directory?).and_return(false)
+        allow(File).to receive(:directory?).with("/etc/heroku").and_return(true)
+      end
+
+      context "when it's on heroku ci" do
+        it "returns nil" do
+          begin
+            original_ci_val = ENV["CI"]
+            ENV["CI"] = "true"
+
+            described_class.init
+            expect(described_class.configuration.release).to eq(nil)
+          ensure
+            ENV["CI"] = original_ci_val
+          end
+        end
+      end
+
+      context "when it's not on heroku ci" do
+        around do |example|
+          begin
+            original_ci_val = ENV["CI"]
+            ENV["CI"] = nil
+
+            example.run
+          ensure
+            ENV["CI"] = original_ci_val
+          end
+        end
+
+        it "returns nil + logs an warning if HEROKU_SLUG_COMMIT is not set" do
+          string_io = StringIO.new
+          logger = Logger.new(string_io)
+
+          described_class.init do |config|
+            config.logger = logger
+          end
+
+          expect(described_class.configuration.release).to eq(nil)
+          expect(string_io.string).to include(Sentry::Configuration::HEROKU_DYNO_METADATA_MESSAGE)
+        end
+
+        it "returns HEROKU_SLUG_COMMIT" do
+          begin
+            ENV["HEROKU_SLUG_COMMIT"] = "REVISION"
+
+            described_class.init
+            expect(described_class.configuration.release).to eq("REVISION")
+          ensure
+            ENV["HEROKU_SLUG_COMMIT"] = nil
+          end
+        end
+      end
+
+      context "when having an error detecting the release" do
+        it "logs the error" do
+          string_io = StringIO.new
+          logger = Logger.new(string_io)
+          allow_any_instance_of(Sentry::Configuration).to receive(:detect_release_from_git).and_raise(TypeError.new)
+
+          described_class.init do |config|
+            config.logger = logger
+          end
+
+          expect(string_io.string).to include("ERROR -- sentry: Error detecting release: TypeError")
+        end
+      end
+    end
+  end
+
 end