Make transaction a required argument of Span

CHANGELOG.md

@@ -24,6 +24,10 @@
 - Add missing `initialized?` checks to `sentry-rails` [#1919](https://github.com/getsentry/sentry-ruby/pull/1919)
   - Fixes [#1885](https://github.com/getsentry/sentry-ruby/issues/1885)
 
+### Refactoring
+
+- Make transaction a required argument of Span [#1921](https://github.com/getsentry/sentry-ruby/pull/1921)
+
 ## 5.5.0
 
 ### Features

sentry-ruby/lib/sentry/span.rb

@@ -60,9 +60,10 @@ class Span
     # The Transaction object the Span belongs to.
     # Every span needs to be attached to a Transaction and their child spans will also inherit the same transaction.
     # @return [Transaction]
-    attr_accessor :transaction
+    attr_reader :transaction
 
     def initialize(
+      transaction:,
       description: nil,
       op: nil,
       status: nil,
@@ -79,6 +80,7 @@ def initialize(
       @start_timestamp = start_timestamp || Sentry.utc_now.to_f
       @timestamp = timestamp
       @description = description
+      @transaction = transaction
       @op = op
       @status = status
       @data = {}
@@ -105,10 +107,10 @@ def to_sentry_trace
     end
 
     # Generates a W3C Baggage header string for distributed tracing
-    # from the incoming baggage stored on the transation.
+    # from the incoming baggage stored on the transaction.
     # @return [String, nil]
     def to_baggage
-      transaction&.get_baggage&.serialize
+      transaction.get_baggage&.serialize
     end
 
     # @return [Hash]
@@ -143,9 +145,8 @@ def get_trace_context
     # Starts a child span with given attributes.
     # @param attributes [Hash] the attributes for the child span.
     def start_child(**attributes)
-      attributes = attributes.dup.merge(trace_id: @trace_id, parent_span_id: @span_id, sampled: @sampled)
+      attributes = attributes.dup.merge(transaction: @transaction, trace_id: @trace_id, parent_span_id: @span_id, sampled: @sampled)
       new_span = Span.new(**attributes)
-      new_span.transaction = transaction
       new_span.span_recorder = span_recorder
 
       if span_recorder

sentry-ruby/lib/sentry/transaction.rb

@@ -58,12 +58,11 @@ def initialize(
       baggage: nil,
       **options
     )
-      super(**options)
+      super(transaction: self, **options)
 
       @name = name
       @source = SOURCES.include?(source) ? source.to_sym : :custom
       @parent_sampled = parent_sampled
-      @transaction = self
       @hub = hub
       @baggage = baggage
       @configuration = hub.configuration # to be removed

sentry-ruby/spec/sentry/client_spec.rb

@@ -17,6 +17,15 @@ def sentry_context
   end
   subject { Sentry::Client.new(configuration) }
 
+  let(:transaction) do
+    hub = Sentry::Hub.new(subject, Sentry::Scope.new)
+    Sentry::Transaction.new(
+      name: "test transaction",
+      hub: hub,
+      sampled: true
+    )
+  end
+
   let(:fake_time) { Time.now }
 
   before do
@@ -432,7 +441,7 @@ module ExcTag; end
       configuration.logger = logger
     end
 
-    let(:span) { Sentry::Span.new }
+    let(:span) { Sentry::Span.new(transaction: transaction) }
 
     it "generates the trace with given span and logs correct message" do
       expect(subject.generate_sentry_trace(span)).to eq(span.to_sentry_trace)

sentry-ruby/spec/sentry/scope/setters_spec.rb

@@ -24,14 +24,26 @@
   end
 
   describe "#set_span" do
-    let(:span) { Sentry::Span.new(op: "foo") }
+    let(:transaction) do
+      client = Sentry::Client.new(Sentry::Configuration.new)
+      hub = Sentry::Hub.new(client, subject)
+      Sentry::Transaction.new(name: "test transaction", hub: hub)
+    end
+
+    let(:span) { Sentry::Span.new(op: "foo", transaction: transaction) }
 
     it "sets the Span" do
       subject.set_span(span)
 
       expect(subject.span).to eq(span)
     end
 
+    it "sets the Transaction" do
+      subject.set_span(transaction)
+
+      expect(subject.span).to eq(transaction)
+    end
+
     it "raises error when passed non-Span argument" do
       expect do
         subject.set_span(1)

sentry-ruby/spec/sentry/scope_spec.rb

@@ -111,18 +111,13 @@
   end
 
   describe "#clear" do
-    subject do
-      scope = described_class.new
-      scope.set_tags({foo: "bar"})
-      scope.set_extras({additional_info: "hello"})
-      scope.set_user({id: 1})
-      scope.set_transaction_name("WelcomeController#index")
-      scope.set_span(Sentry::Span.new)
-      scope.set_fingerprint(["foo"])
-      scope
-    end
-
     it "resets the scope's data" do
+      subject.set_tags({foo: "bar"})
+      subject.set_extras({additional_info: "hello"})
+      subject.set_user({id: 1})
+      subject.set_transaction_name("WelcomeController#index")
+      subject.set_span(Sentry::Transaction.new(op: "foo", hub: hub))
+      subject.set_fingerprint(["foo"])
       scope_id = subject.object_id
 
       subject.clear
@@ -241,12 +236,12 @@
     end
 
     it "sets trace context if there's a span" do
-      span = Sentry::Span.new(op: "foo")
-      subject.set_span(span)
+      transaction = Sentry::Transaction.new(op: "foo", hub: hub)
+      subject.set_span(transaction)
 
       subject.apply_to_event(event)
 
-      expect(event.contexts[:trace]).to eq(span.get_trace_context)
+      expect(event.contexts[:trace]).to eq(transaction.get_trace_context)
       expect(event.contexts.dig(:trace, :op)).to eq("foo")
     end
 

sentry-ruby/spec/sentry/span_spec.rb

@@ -1,8 +1,22 @@
 require "spec_helper"
 
 RSpec.describe Sentry::Span do
+  let(:hub) do
+    client = Sentry::Client.new(Sentry::Configuration.new)
+    Sentry::Hub.new(client, Sentry::Scope.new)
+  end
+
+  let(:transaction) do
+    Sentry::Transaction.new(
+      name: "test transaction",
+      hub: hub,
+      sampled: true
+    )
+  end
+
   subject do
     described_class.new(
+      transaction: transaction,
       op: "sql.query",
       description: "SELECT * FROM users;",
       status: "ok",
@@ -67,7 +81,7 @@
     end
 
     context "without sampled value" do
-      subject { described_class.new }
+      subject { described_class.new(transaction: transaction) }
 
       it "doesn't contain the sampled flag" do
         sentry_trace = subject.to_sentry_trace
@@ -129,6 +143,16 @@
       expect(new_span.sampled).to eq(true)
     end
 
+    it "gives the child span its transaction" do
+      span_1 = subject.start_child
+
+      expect(span_1.transaction).to eq(subject.transaction)
+
+      span_2 = span_1.start_child
+
+      expect(span_2.transaction).to eq(subject.transaction)
+    end
+
     context "when the parent span has a span_recorder" do
       subject do
         # inherits the span recorder from the transaction
@@ -150,22 +174,6 @@
         expect(subject.span_recorder.spans.count).to eq(4)
       end
     end
-
-    context "when the parent span has a transaction" do
-      before do
-        subject.transaction = Sentry::Transaction.new(hub: Sentry.get_current_hub)
-      end
-
-      it "gives the child span its transaction" do
-        span_1 = subject.start_child
-
-        expect(span_1.transaction).to eq(subject.transaction)
-
-        span_2 = span_1.start_child
-
-        expect(span_2.transaction).to eq(subject.transaction)
-      end
-    end
   end
 
   describe "#with_child_span" do

sentry-ruby/spec/sentry_spec.rb

@@ -494,7 +494,10 @@
     end
 
     context "when the current span is present" do
-      let(:parent_span) { Sentry::Span.new(op: "parent") }
+      let(:parent_span) do
+        transaction = Sentry::Transaction.new(op: "foo", hub: Sentry.get_current_hub)
+        Sentry::Span.new(op: "parent", transaction: transaction)
+      end
 
       before do
         described_class.get_current_scope.set_span(parent_span)