Refactor tracing implementation

sentry-rails/lib/sentry/rails/capture_exceptions.rb

@@ -29,11 +29,14 @@ def capture_exception(exception)
         Sentry::Rails.capture_exception(exception)
       end
 
-      def finish_span(span, status_code)
-        if @assets_regex.nil? || !span.name.match?(@assets_regex)
-          span.set_http_status(status_code)
-          span.finish
+      def start_transaction(env, scope)
+        transaction = super
+
+        if @assets_regex && transaction.name.match?(@assets_regex)
+          transaction.instance_variable_set(:@sampled, false)
         end
+
+        transaction
       end
     end
   end

sentry-ruby/CHANGELOG.md

@@ -5,6 +5,7 @@
 - Refactor interface construction [#1296](https://github.com/getsentry/sentry-ruby/pull/1296)
 - Ignore sentry-trace when tracing is not enabled [#1308](https://github.com/getsentry/sentry-ruby/pull/1308)
   - Fixes [#1307](https://github.com/getsentry/sentry-ruby/issues/1307)
+- Refactor tracing implementation [#1309](https://github.com/getsentry/sentry-ruby/pull/1309)
 
 ## 4.2.2
 

sentry-ruby/lib/sentry/hub.rb

@@ -69,9 +69,9 @@ def pop_scope
       @stack.pop
     end
 
-    def start_transaction(transaction: nil, **options)
+    def start_transaction(transaction: nil, configuration: Sentry.configuration, **options)
       transaction ||= Transaction.new(**options)
-      transaction.set_initial_sample_desicion
+      transaction.set_initial_sample_decision(configuration: current_client.configuration)
       transaction
     end
 

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

@@ -16,27 +16,24 @@ def call(env)
           scope.set_transaction_name(env["PATH_INFO"]) if env["PATH_INFO"]
           scope.set_rack_env(env)
 
-          sentry_trace = env["HTTP_SENTRY_TRACE"]
-          span = Sentry::Transaction.from_sentry_trace(sentry_trace, name: scope.transaction_name, op: transaction_op) if sentry_trace
-          span ||= Sentry.start_transaction(name: scope.transaction_name, op: transaction_op)
-
-          scope.set_span(span)
+          transaction = start_transaction(env, scope)
+          scope.set_span(transaction)
 
           begin
             response = @app.call(env)
           rescue Sentry::Error
-            finish_span(span, 500)
+            finish_transaction(transaction, 500)
             raise # Don't capture Sentry errors
           rescue Exception => e
             capture_exception(e)
-            finish_span(span, 500)
+            finish_transaction(transaction, 500)
             raise
           end
 
           exception = collect_exception(env)
           capture_exception(exception) if exception
 
-          finish_span(span, response[0])
+          finish_transaction(transaction, response[0])
 
           response
         end
@@ -56,9 +53,16 @@ def capture_exception(exception)
         Sentry.capture_exception(exception)
       end
 
-      def finish_span(span, status_code)
-        span.set_http_status(status_code)
-        span.finish
+      def start_transaction(env, scope)
+        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)
+      end
+
+
+      def finish_transaction(transaction, status_code)
+        transaction.set_http_status(status_code)
+        transaction.finish
       end
     end
   end

sentry-ruby/lib/sentry/transaction.rb

@@ -25,8 +25,8 @@ def set_span_recorder
       @span_recorder.add(self)
     end
 
-    def self.from_sentry_trace(sentry_trace, **options)
-      return unless Sentry.configuration.tracing_enabled?
+    def self.from_sentry_trace(sentry_trace, configuration: Sentry.configuration, **options)
+      return unless configuration.tracing_enabled?
       return unless sentry_trace
 
       match = SENTRY_TRACE_REGEXP.match(sentry_trace)
@@ -68,8 +68,8 @@ def deep_dup
       copy
     end
 
-    def set_initial_sample_desicion(sampling_context = {})
-      unless Sentry.configuration.tracing_enabled?
+    def set_initial_sample_decision(sampling_context: {}, configuration: Sentry.configuration)
+      unless configuration.tracing_enabled?
         @sampled = false
         return
       end
@@ -78,9 +78,9 @@ def set_initial_sample_desicion(sampling_context = {})
 
       transaction_description = generate_transaction_description
 
-      logger = Sentry.configuration.logger
-      sample_rate = Sentry.configuration.traces_sample_rate
-      traces_sampler = Sentry.configuration.traces_sampler
+      logger = configuration.logger
+      sample_rate = configuration.traces_sample_rate
+      traces_sampler = configuration.traces_sampler
 
       if traces_sampler.is_a?(Proc)
         sampling_context = sampling_context.merge(

sentry-ruby/spec/sentry/transaction_spec.rb

@@ -15,15 +15,17 @@
   describe ".from_sentry_trace" do
     let(:sentry_trace) { subject.to_sentry_trace }
 
-    before do
-      configuration = Sentry::Configuration.new
-      configuration.traces_sample_rate = 1.0
-      allow(Sentry).to receive(:configuration).and_return(configuration)
+    let(:configuration) do
+      Sentry::Configuration.new
     end
 
     context "when tracing is enabled" do
+      before do
+        configuration.traces_sample_rate = 1.0
+      end
+
       it "returns correctly-formatted value" do
-        child_transaction = described_class.from_sentry_trace(sentry_trace, op: "child")
+        child_transaction = described_class.from_sentry_trace(sentry_trace, op: "child", configuration: configuration)
 
         expect(child_transaction.trace_id).to eq(subject.trace_id)
         expect(child_transaction.parent_span_id).to eq(subject.span_id)
@@ -33,20 +35,19 @@
       end
 
       it "handles invalid values without crashing" do
-        child_transaction = described_class.from_sentry_trace("dummy", op: "child")
+        child_transaction = described_class.from_sentry_trace("dummy", op: "child", configuration: configuration)
 
         expect(child_transaction).to be_nil
       end
     end
 
     context "when tracing is disabled" do
       before do
-        configuration = Sentry::Configuration.new
-        allow(Sentry).to receive(:configuration).and_return(configuration)
+        configuration.traces_sample_rate = 0.0
       end
 
       it "returns nil" do
-        expect(described_class.from_sentry_trace(sentry_trace, op: "child")).to be_nil
+        expect(described_class.from_sentry_trace(sentry_trace, op: "child", configuration: configuration)).to be_nil
       end
     end
   end
@@ -121,7 +122,7 @@
     end
   end
 
-  describe "#set_initial_sample_desicion" do
+  describe "#set_initial_sample_decision" do
     before do
       perform_basic_setup
     end
@@ -135,7 +136,7 @@
         allow(Sentry.configuration).to receive(:tracing_enabled?).and_return(false)
 
         transaction = described_class.new(sampled: true)
-        transaction.set_initial_sample_desicion
+        transaction.set_initial_sample_decision
         expect(transaction.sampled).to eq(false)
       end
     end
@@ -150,11 +151,11 @@
       context "when the transaction already has a decision" do
         it "doesn't change it" do
           transaction = described_class.new(sampled: true)
-          transaction.set_initial_sample_desicion
+          transaction.set_initial_sample_decision
           expect(transaction.sampled).to eq(true)
 
           transaction = described_class.new(sampled: false)
-          transaction.set_initial_sample_desicion
+          transaction.set_initial_sample_decision
           expect(transaction.sampled).to eq(false)
         end
       end
@@ -170,7 +171,7 @@
             "[Tracing] Starting <rack.request> transaction"
           )
 
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(true)
         end
 
@@ -180,14 +181,14 @@
             "[Tracing] Discarding <rack.request> transaction because it's not included in the random sample (sampling rate = 0.5)"
           )
 
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(false)
         end
 
         it "accepts integer traces_sample_rate" do
           Sentry.configuration.traces_sample_rate = 1
 
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(true)
         end
       end
@@ -197,7 +198,7 @@
           Sentry.configuration.traces_sampler = ""
 
           expect do
-            subject.set_initial_sample_desicion
+            subject.set_initial_sample_decision
           end.not_to raise_error
         end
 
@@ -208,7 +209,7 @@
             sampling_context = context
           end
 
-          subject.set_initial_sample_desicion(foo: "bar")
+          subject.set_initial_sample_decision(sampling_context: { foo: "bar" })
 
           # transaction_context's sampled attribute will be the old value
           expect(sampling_context[:transaction_context].keys).to eq(subject.to_hash.keys)
@@ -222,7 +223,7 @@
           )
 
           Sentry.configuration.traces_sampler = -> (_) { "foo" }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
 
           expect(subject.sampled).to eq(false)
         end
@@ -234,17 +235,17 @@
 
           subject = described_class.new
           Sentry.configuration.traces_sampler = -> (_) { true }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(true)
 
           subject = described_class.new
           Sentry.configuration.traces_sampler = -> (_) { 1.0 }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(true)
 
           subject = described_class.new
           Sentry.configuration.traces_sampler = -> (_) { 1 }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(true)
         end
 
@@ -255,12 +256,12 @@
 
           subject = described_class.new
           Sentry.configuration.traces_sampler = -> (_) { false }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(false)
 
           subject = described_class.new
           Sentry.configuration.traces_sampler = -> (_) { 0.0 }
-          subject.set_initial_sample_desicion
+          subject.set_initial_sample_decision
           expect(subject.sampled).to eq(false)
         end
       end