diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb index 8bb7efbbc..c45730c91 100644 --- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb +++ b/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 diff --git a/sentry-ruby/CHANGELOG.md b/sentry-ruby/CHANGELOG.md index 375d69308..8013e70e4 100644 --- a/sentry-ruby/CHANGELOG.md +++ b/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 diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index a421596f7..e86ece913 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/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 diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index f664c7331..5d8b5afbb 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/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 diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index f47b29a1b..8d22323a5 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/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( diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb index 336d57936..be7d2cffe 100644 --- a/sentry-ruby/spec/sentry/transaction_spec.rb +++ b/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,7 +35,7 @@ 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 @@ -41,12 +43,11 @@ 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 transaction" ) - subject.set_initial_sample_desicion + subject.set_initial_sample_decision expect(subject.sampled).to eq(true) end @@ -180,14 +181,14 @@ "[Tracing] Discarding 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