Turn SampleRand into a class

solnic · solnic · commit 763c8add58b7 · 2025-08-13T13:42:14.000Z
diff --git a/sentry-ruby/lib/sentry/propagation_context.rb b/sentry-ruby/lib/sentry/propagation_context.rb
@@ -146,22 +146,24 @@ def extract_sample_rand_from_baggage(baggage)
       sample_rand_str = baggage.items["sample_rand"]
       return unless sample_rand_str
 
-      sample_rand = sample_rand_str.to_f
-      Utils::SampleRand.valid?(sample_rand) ? sample_rand : nil
+      generator = Utils::SampleRand.new(trace_id: @trace_id)
+      generator.generate_from_value(sample_rand_str)
     end
 
     def generate_sample_rand
+      generator = Utils::SampleRand.new(trace_id: @trace_id)
+
       if @incoming_trace && !@parent_sampled.nil? && @baggage
         sample_rate_str = @baggage.items["sample_rate"]
         sample_rate = sample_rate_str&.to_f
 
         if sample_rate && !@parent_sampled.nil?
-          Utils::SampleRand.generate_from_sampling_decision(@parent_sampled, sample_rate, @trace_id)
+          generator.generate_from_sampling_decision(@parent_sampled, sample_rate)
         else
-          Utils::SampleRand.generate_from_trace_id(@trace_id)
+          generator.generate_from_trace_id
         end
       else
-        Utils::SampleRand.generate_from_trace_id(@trace_id)
+        generator.generate_from_trace_id
       end
     end
   end
diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb
@@ -96,7 +96,10 @@ def initialize(
 
       init_span_recorder
 
-      @sample_rand ||= Utils::SampleRand.generate_from_trace_id(@trace_id)
+      unless @sample_rand
+        generator = Utils::SampleRand.new(trace_id: @trace_id)
+        @sample_rand = generator.generate_from_trace_id
+      end
     end
 
     # @deprecated use Sentry.continue_trace instead.
@@ -152,20 +155,25 @@ def self.extract_sentry_trace(sentry_trace)
     end
 
     def self.extract_sample_rand_from_baggage(baggage, trace_id, parent_sampled)
-      return Utils::SampleRand.generate_from_trace_id(trace_id) unless baggage&.items
+      generator = Utils::SampleRand.new(trace_id: trace_id)
+
+      unless baggage&.items
+        return generator.generate_from_trace_id
+      end
 
       sample_rand_str = baggage.items["sample_rand"]
-      if sample_rand_str && Utils::SampleRand.valid?(sample_rand_str.to_f)
-        sample_rand_str.to_f
-      else
-        sample_rate_str = baggage.items["sample_rate"]
-        sample_rate = sample_rate_str&.to_f
 
-        if sample_rate && parent_sampled != nil
-          Utils::SampleRand.generate_from_sampling_decision(parent_sampled, sample_rate, trace_id)
-        else
-          Utils::SampleRand.generate_from_trace_id(trace_id)
-        end
+      if sample_rand_str
+        return generator.generate_from_value(sample_rand_str)
+      end
+
+      sample_rate_str = baggage.items["sample_rate"]
+      sample_rate = sample_rate_str&.to_f
+
+      if sample_rate && parent_sampled != nil
+        generator.generate_from_sampling_decision(parent_sampled, sample_rate)
+      else
+        generator.generate_from_trace_id
       end
     end
 
diff --git a/sentry-ruby/lib/sentry/utils/sample_rand.rb b/sentry-ruby/lib/sentry/utils/sample_rand.rb
@@ -2,46 +2,96 @@
 
 module Sentry
   module Utils
-    module SampleRand
-      def self.generate_from_trace_id(trace_id)
-        (random_from_trace_id(trace_id) * 1_000_000).floor / 1_000_000.0
+    class SampleRand
+      PRECISION = 1_000_000.0
+      FORMAT_PRECISION = 6
+
+      attr_reader :trace_id
+
+      def self.valid?(value)
+        return false unless value
+        value >= 0.0 && value < 1.0
+      end
+
+      def self.format(value)
+        return unless value
+
+        truncated = (value * PRECISION).floor / PRECISION
+        "%.#{FORMAT_PRECISION}f" % truncated
+      end
+
+      def initialize(trace_id: nil)
+        @trace_id = trace_id
       end
 
-      def self.generate_from_sampling_decision(sampled, sample_rate, trace_id = nil)
-        if sample_rate.nil? || sample_rate <= 0.0 || sample_rate > 1.0
-          trace_id ? generate_from_trace_id(trace_id) : format(Random.rand(1.0)).to_f
+      def generate_from_trace_id
+        (random_from_trace_id * PRECISION).floor / PRECISION
+      end
+
+      def generate_from_sampling_decision(sampled, sample_rate)
+        if invalid_sample_rate?(sample_rate)
+          fallback_generation
         else
-          random = random_from_trace_id(trace_id)
-
-          if sampled
-            format(random * sample_rate)
-          elsif sample_rate == 1.0
-            format(random)
-          else
-            format(sample_rate + random * (1.0 - sample_rate))
-          end.to_f
+          generate_based_on_sampling(sampled, sample_rate)
         end
       end
 
-      def self.random_from_trace_id(trace_id)
-        if trace_id
-          Random.new(trace_id[0, 16].to_i(16))
+      def generate_from_value(sample_rand_value)
+        parsed_value = parse_value(sample_rand_value)
+
+        if self.class.valid?(parsed_value)
+          parsed_value
+        else
+          fallback_generation
+        end
+      end
+
+      private
+
+      def random_from_trace_id
+        if @trace_id
+          Random.new(@trace_id[0, 16].to_i(16))
         else
           Random.new
         end.rand(1.0)
       end
 
-      def self.valid?(sample_rand)
-        return false unless sample_rand
-        return false if sample_rand.is_a?(String) && sample_rand.empty?
+      def invalid_sample_rate?(sample_rate)
+        sample_rate.nil? || sample_rate <= 0.0 || sample_rate > 1.0
+      end
 
-        value = sample_rand.is_a?(String) ? sample_rand.to_f : sample_rand
-        value >= 0.0 && value < 1.0
+      def fallback_generation
+        if @trace_id
+          (random_from_trace_id * PRECISION).floor / PRECISION
+        else
+          format_random(Random.rand(1.0))
+        end
+      end
+
+      def generate_based_on_sampling(sampled, sample_rate)
+        random = random_from_trace_id
+
+        result = if sampled
+          random * sample_rate
+        elsif sample_rate == 1.0
+          random
+        else
+          sample_rate + random * (1.0 - sample_rate)
+        end
+
+        format_random(result)
       end
 
-      def self.format(sample_rand)
-        truncated = (sample_rand * 1_000_000).floor / 1_000_000.0
-        "%.6f" % truncated
+      def format_random(value)
+        truncated = (value * PRECISION).floor / PRECISION
+        ("%.#{FORMAT_PRECISION}f" % truncated).to_f
+      end
+
+      def parse_value(sample_rand_value)
+        return unless sample_rand_value
+        return if sample_rand_value.is_a?(String) && sample_rand_value.empty?
+
+        sample_rand_value.is_a?(String) ? sample_rand_value.to_f : sample_rand_value
       end
     end
   end
diff --git a/sentry-ruby/spec/sentry/hub_spec.rb b/sentry-ruby/spec/sentry/hub_spec.rb
@@ -715,7 +715,8 @@
 
           expect(transaction.sample_rand).to eq(propagation_context.sample_rand)
 
-          expected = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+          generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+          expected = generator.generate_from_trace_id
 
           expect(transaction.sample_rand).to eq(expected)
         end
diff --git a/sentry-ruby/spec/sentry/propagation_context/sample_rand_spec.rb b/sentry-ruby/spec/sentry/propagation_context/sample_rand_spec.rb
@@ -103,8 +103,11 @@
         it "uses parent's explicit unsampled decision instead of falling back to trace_id generation" do
           context = described_class.new(scope, env)
 
-          expected_from_decision = Sentry::Utils::SampleRand.generate_from_sampling_decision(false, 0.5, "771a43a4192642f0b136d5159a501700")
-          expected_from_trace_id = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+          generator1 = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+          expected_from_decision = generator1.generate_from_sampling_decision(false, 0.5)
+
+          generator2 = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+          expected_from_trace_id = generator2.generate_from_trace_id
 
           expect(context.sample_rand).to eq(expected_from_decision)
           expect(context.sample_rand).not_to eq(expected_from_trace_id)
@@ -126,7 +129,8 @@
           expect(context.sample_rand).to be < 1.0
           expect(context.incoming_trace).to be true
 
-          expected = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+          generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+          expected = generator.generate_from_trace_id
           expect(context.sample_rand).to eq(expected)
         end
       end
diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
@@ -279,11 +279,11 @@ def verify_transaction_doesnt_inherit_external_transaction(transaction, external
       end
 
       def wont_be_sampled_by_sdk
-        allow(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(1.0)
+        allow_any_instance_of(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(1.0)
       end
 
       def will_be_sampled_by_sdk
-        allow(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.3)
+        allow_any_instance_of(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.3)
       end
 
       before do
@@ -430,7 +430,7 @@ def will_be_sampled_by_sdk
 
     context "when the transaction is sampled" do
       before do
-        allow(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.4)
+        allow_any_instance_of(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.4)
       end
 
       it "starts a transaction and finishes it" do
@@ -488,7 +488,7 @@ def will_be_sampled_by_sdk
 
     context "when the transaction is not sampled" do
       before do
-        allow(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(1.0)
+        allow_any_instance_of(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(1.0)
       end
 
       it "doesn't do anything" do
@@ -506,7 +506,7 @@ def will_be_sampled_by_sdk
 
     context "when there's an exception" do
       before do
-        allow(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.4)
+        allow_any_instance_of(Sentry::Utils::SampleRand).to receive(:generate_from_trace_id).and_return(0.4)
       end
 
       it "still finishes the transaction" do
diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb
@@ -658,6 +658,46 @@
     end
   end
 
+  describe ".extract_sample_rand_from_baggage" do
+    let(:trace_id) { "771a43a4192642f0b136d5159a501700" }
+
+    it "returns trace_id generation when baggage is nil" do
+      result = described_class.extract_sample_rand_from_baggage(nil, trace_id, true)
+
+      generator = Sentry::Utils::SampleRand.new(trace_id: trace_id)
+      expected = generator.generate_from_trace_id
+
+      expect(result).to eq(expected)
+    end
+
+    it "returns trace_id generation when baggage has no items" do
+      baggage = double("baggage", items: nil)
+      result = described_class.extract_sample_rand_from_baggage(baggage, trace_id, true)
+
+      generator = Sentry::Utils::SampleRand.new(trace_id: trace_id)
+      expected = generator.generate_from_trace_id
+
+      expect(result).to eq(expected)
+    end
+
+    it "returns trace_id generation when sample_rand is invalid" do
+      baggage = double("baggage", items: { "sample_rand" => "1.5" })
+      result = described_class.extract_sample_rand_from_baggage(baggage, trace_id, true)
+
+      generator = Sentry::Utils::SampleRand.new(trace_id: trace_id)
+      expected = generator.generate_from_trace_id
+
+      expect(result).to eq(expected)
+    end
+
+    it "returns valid sample_rand from baggage when present" do
+      baggage = double("baggage", items: { "sample_rand" => "0.5" })
+      result = described_class.extract_sample_rand_from_baggage(baggage, trace_id, true)
+
+      expect(result).to eq(0.5)
+    end
+  end
+
   describe "#set_name" do
     it "sets name and source directly" do
       subject.set_name("bar", source: :url)
diff --git a/sentry-ruby/spec/sentry/transactions/sample_rand_propagation_spec.rb b/sentry-ruby/spec/sentry/transactions/sample_rand_propagation_spec.rb
@@ -34,7 +34,8 @@
 
       expect(transaction.sample_rand).to eq(propagation_context.sample_rand)
 
-      expected = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+      generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+      expected = generator.generate_from_trace_id
 
       expect(transaction.sample_rand).to eq(expected)
       expect(propagation_context.sample_rand).to eq(expected)
@@ -90,7 +91,8 @@
       transaction = Sentry.continue_trace(env, name: "test")
       propagation_context = Sentry.get_current_scope.propagation_context
 
-      expected = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+      generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+      expected = generator.generate_from_trace_id
 
       expect(transaction.sample_rand).to eq(expected)
       expect(propagation_context.sample_rand).to eq(expected)
diff --git a/sentry-ruby/spec/sentry/transactions/trace_propagation_spec.rb b/sentry-ruby/spec/sentry/transactions/trace_propagation_spec.rb
@@ -68,19 +68,15 @@
 
       expect(transaction).not_to be_nil
 
-      expected_sample_rand = Sentry::Utils::SampleRand.generate_from_sampling_decision(
-        true,
-        0.25,
-        "771a43a4192642f0b136d5159a501700"
-      )
+      generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+      expected_sample_rand = generator.generate_from_sampling_decision(true, 0.25)
 
       expect(expected_sample_rand).to be >= 0.0
       expect(expected_sample_rand).to be < 1.0
       expect(expected_sample_rand).to be < 0.25
 
-      expected_sample_rand2 = Sentry::Utils::SampleRand.generate_from_sampling_decision(
-        true, 0.25, "771a43a4192642f0b136d5159a501700"
-      )
+      generator2 = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+      expected_sample_rand2 = generator2.generate_from_sampling_decision(true, 0.25)
       expect(expected_sample_rand2).to eq(expected_sample_rand)
 
       baggage = transaction.get_baggage
@@ -125,9 +121,9 @@
 
       expect(sample_rands.uniq.length).to eq(1)
 
-      expected_sample_rand = Sentry::Utils::SampleRand.format(
-        Sentry::Utils::SampleRand.generate_from_trace_id(trace_id)
-      )
+      generator = Sentry::Utils::SampleRand.new(trace_id: trace_id)
+      value = generator.generate_from_trace_id
+      expected_sample_rand = Sentry::Utils::SampleRand.format(value)
       expect(sample_rands.first).to eq(expected_sample_rand)
     end
 
@@ -165,7 +161,8 @@
 
       expect(transaction).not_to be_nil
 
-      expected_sample_rand = Sentry::Utils::SampleRand.generate_from_trace_id("771a43a4192642f0b136d5159a501700")
+      generator = Sentry::Utils::SampleRand.new(trace_id: "771a43a4192642f0b136d5159a501700")
+      expected_sample_rand = generator.generate_from_trace_id
 
       expect(expected_sample_rand).to be >= 0.0
       expect(expected_sample_rand).to be < 1.0
diff --git a/sentry-ruby/spec/sentry/utils/sample_rand_spec.rb b/sentry-ruby/spec/sentry/utils/sample_rand_spec.rb
diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb
