fix(tracing): Never send transactions if tracing is disabled

sentry-ruby/lib/sentry-ruby.rb

@@ -164,6 +164,8 @@ def capture_event(event)
 
     # Takes or initializes a new Sentry::Transaction and makes a sampling decision for it.
     def start_transaction(**options)
+      return unless configuration.tracing_enabled?
+
       get_current_hub&.start_transaction(**options)
     end
 

sentry-ruby/lib/sentry/configuration.rb

@@ -293,7 +293,7 @@ def enabled_in_current_env?
     end
 
     def tracing_enabled?
-      !!((@traces_sample_rate && @traces_sample_rate > 0.0) || @traces_sampler)
+      !!(@traces_sampler || (@traces_sample_rate.is_a?(Numeric) && @traces_sample_rate >= 0.0 && @traces_sample_rate <= 1.0))
     end
 
     def stacktrace_builder

sentry-ruby/lib/sentry/hub.rb

@@ -70,6 +70,8 @@ def pop_scope
     end
 
     def start_transaction(transaction: nil, configuration: Sentry.configuration, **options)
+      return unless configuration.tracing_enabled?
+
       transaction ||= Transaction.new(**options)
       transaction.set_initial_sample_decision(configuration: current_client.configuration)
       transaction

sentry-ruby/lib/sentry/rack/capture_exceptions.rb

@@ -17,23 +17,23 @@ def call(env)
           scope.set_rack_env(env)
 
           transaction = start_transaction(env, scope)
-          scope.set_span(transaction)
+          transaction ? scope.set_span(transaction) : nil
 
           begin
             response = @app.call(env)
           rescue Sentry::Error
-            finish_transaction(transaction, 500)
+            transaction ? finish_transaction(transaction, 500) : nil
             raise # Don't capture Sentry errors
           rescue Exception => e
             capture_exception(e)
-            finish_transaction(transaction, 500)
+            transaction ? finish_transaction(transaction, 500) : nil
             raise
           end
 
           exception = collect_exception(env)
           capture_exception(exception) if exception
 
-          finish_transaction(transaction, response[0])
+          transaction ? finish_transaction(transaction, response[0]) : nil
 
           response
         end
@@ -54,13 +54,20 @@ def capture_exception(exception)
       end
 
       def start_transaction(env, scope)
+        return unless Sentry.configuration.tracing_enabled?
+
         sentry_trace = env["HTTP_SENTRY_TRACE"]
-        transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, name: scope.transaction_name, op: transaction_op) if sentry_trace
-        transaction || Sentry.start_transaction(name: scope.transaction_name, op: transaction_op)
+        options = {name: scope.transaction_name, op: transaction_op}
+
+        # if tracing is disabled, these will both return nil
+        transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
+        Sentry.start_transaction(transaction: transaction, **options)
       end
 
 
       def finish_transaction(transaction, status_code)
+        return unless transaction
+
         transaction.set_http_status(status_code)
         transaction.finish
       end

sentry-ruby/lib/sentry/transaction.rb

@@ -33,9 +33,16 @@ def self.from_sentry_trace(sentry_trace, configuration: Sentry.configuration, **
       return if match.nil?
       trace_id, parent_span_id, sampled_flag = match[1..3]
 
-      sampled = sampled_flag != "0"
+      if sampled_flag == "1"
+        parent_sampled = true
+      elsif sampled_flag == "0"
+        parent_sampled = false
+      else
+        parent_sampled = nil
+      end
 
-      new(trace_id: trace_id, parent_span_id: parent_span_id, parent_sampled: sampled, sampled: sampled, **options)
+
+      new(trace_id: trace_id, parent_span_id: parent_span_id, parent_sampled: parent_sampled, **options)
     end
 
     def to_hash
@@ -69,26 +76,28 @@ def deep_dup
     end
 
     def set_initial_sample_decision(sampling_context: {}, configuration: Sentry.configuration)
+      logger = configuration.logger
+      transaction_description = generate_transaction_description
+
       unless configuration.tracing_enabled?
-        @sampled = false
-        return
+        # we shouldn't ever get here (because if tracing is disabled, start_transaction should bail
+        # before it calls set_initial_sample_decision), so warn, because something's gone wrong
+        logger.warn("#{MESSAGE_PREFIX} Tracing is disabled. #{transaction_description} will not be sent.")
+        return nil
       end
 
       return unless @sampled.nil?
 
-      transaction_description = generate_transaction_description
-
-      logger = configuration.logger
-      sample_rate = configuration.traces_sample_rate
-      traces_sampler = configuration.traces_sampler
-
-      if traces_sampler.is_a?(Proc)
+      if configuration.traces_sampler.is_a?(Proc)
         sampling_context = sampling_context.merge(
           parent_sampled: @parent_sampled,
           transaction_context: self.to_hash
         )
-
-        sample_rate = traces_sampler.call(sampling_context)
+        sample_rate = configuration.traces_sampler.call(sampling_context)
+      elsif @parent_sampled
+        sample_rate = @parent_sampled
+      else
+        sample_rate = configuration.traces_sample_rate
       end
 
       unless [true, false].include?(sample_rate) || (sample_rate.is_a?(Numeric) && sample_rate >= 0.0 && sample_rate <= 1.0)
@@ -99,7 +108,7 @@ def set_initial_sample_decision(sampling_context: {}, configuration: Sentry.conf
 
       if sample_rate == 0.0 || sample_rate == false
         @sampled = false
-        logger.debug("#{MESSAGE_PREFIX} Discarding #{transaction_description} because traces_sampler returned 0 or false")
+        logger.debug("#{MESSAGE_PREFIX} Discarding #{transaction_description} because traces_sample_rate is 0 or traces_sampler returned 0 or false")
         return
       end
 
@@ -119,14 +128,16 @@ def set_initial_sample_decision(sampling_context: {}, configuration: Sentry.conf
     end
 
     def finish(hub: nil)
+      # it's unlikely we'd get this far if tracing is disabled, but just in case...
+      return unless Sentry.configuration.tracing_enabled?
+      return unless @sampled
+
       super() # Span#finish doesn't take arguments
 
       if @name.nil?
         @name = UNLABELD_NAME
       end
 
-      return unless @sampled || @parent_sampled
-
       hub ||= Sentry.get_current_hub
       event = hub.current_client.event_from_transaction(self)
       hub.capture_event(event)

sentry-ruby/spec/sentry/configuration_spec.rb

@@ -11,26 +11,36 @@
   end
 
   describe "#tracing_enabled?" do
-    it "returns false by default" do
-      expect(subject.tracing_enabled?).to eq(false)
+    context "when neither traces_sample_rate nor traces_sampler is set" do
+      it "returns false" do
+        expect(subject.tracing_enabled?).to eq(false)
+      end
     end
 
     context "when traces_sample_rate == 0.0" do
-      it "returns false" do
+      it "returns true" do
         subject.traces_sample_rate = 0.0
 
-        expect(subject.tracing_enabled?).to eq(false)
+        expect(subject.tracing_enabled?).to eq(true)
       end
     end
 
-    context "when traces_sample_rate > 0" do
+    context "when traces_sample_rate > 0 and <= 1" do
       it "returns true" do
         subject.traces_sample_rate = 0.1
 
         expect(subject.tracing_enabled?).to eq(true)
       end
     end
 
+    context "when traces_sample_rate > 1" do
+      it "returns false" do
+        subject.traces_sample_rate = 1.1
+
+        expect(subject.tracing_enabled?).to eq(false)
+      end
+    end
+
     context "when traces_sampler is set" do
       it "returns true" do
         subject.traces_sampler = proc { true }

sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb

@@ -182,6 +182,7 @@
 
         stack.call(env)
 
+
         transaction = transport.events.last
         expect(transaction.type).to eq("transaction")
         expect(transaction.timestamp).not_to be_nil
@@ -215,7 +216,7 @@
         allow(Random).to receive(:rand).and_return(0.4)
       end
 
-      it "starts a span and finishes it" do
+      it "starts a transaction and finishes it" do
         app = ->(_) do
           [200, {}, ["ok"]]
         end
@@ -282,9 +283,9 @@
       end
     end
 
-    context "when traces_sample_rate is not set" do
+    context "when tracing is disabled" do
       before do
-        Sentry.configuration.traces_sample_rate = nil
+        allow(Sentry.configuration).to receive(:tracing_enabled?).and_return(false)
       end
 
       let(:stack) do