Skip to content

Propagated sampling rates #2671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Unreleased

### Feature

- Propagated sampling rates as specified in [Traces](https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value) docs ([#2671](https://github.com/getsentry/sentry-ruby/pull/2671))

### Internal

- Factor out do_request in HTTP transport ([#2662](https://github.com/getsentry/sentry-ruby/pull/2662))
Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/lib/sentry-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
require "sentry/utils/argument_checking_helper"
require "sentry/utils/encoding_helper"
require "sentry/utils/logging_helper"
require "sentry/utils/sample_rand"
require "sentry/configuration"
require "sentry/structured_logger"
require "sentry/event"
Expand Down
4 changes: 3 additions & 1 deletion sentry-ruby/lib/sentry/hub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ def start_transaction(transaction: nil, custom_sampling_context: {}, instrumente

sampling_context = {
transaction_context: transaction.to_hash,
parent_sampled: transaction.parent_sampled
parent_sampled: transaction.parent_sampled,
parent_sample_rate: transaction.parent_sample_rate
}

sampling_context.merge!(custom_sampling_context)
Expand Down Expand Up @@ -357,6 +358,7 @@ def continue_trace(env, **options)
parent_span_id: propagation_context.parent_span_id,
parent_sampled: propagation_context.parent_sampled,
baggage: propagation_context.baggage,
sample_rand: propagation_context.sample_rand,
**options
)
end
Expand Down
37 changes: 36 additions & 1 deletion sentry-ruby/lib/sentry/propagation_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "securerandom"
require "sentry/baggage"
require "sentry/utils/uuid"
require "sentry/utils/sample_rand"

module Sentry
class PropagationContext
Expand Down Expand Up @@ -33,13 +34,17 @@ class PropagationContext
# Please use the #get_baggage method for interfacing outside this class.
# @return [Baggage, nil]
attr_reader :baggage
# The propagated random value used for sampling decisions.
# @return [Float, nil]
attr_reader :sample_rand

def initialize(scope, env = nil)
@scope = scope
@parent_span_id = nil
@parent_sampled = nil
@baggage = nil
@incoming_trace = false
@sample_rand = nil

if env
sentry_trace_header = env["HTTP_SENTRY_TRACE"] || env[SENTRY_TRACE_HEADER_NAME]
Expand All @@ -61,6 +66,7 @@ def initialize(scope, env = nil)
Baggage.new({})
end

@sample_rand = extract_sample_rand_from_baggage(@baggage)
@baggage.freeze!
@incoming_trace = true
end
Expand All @@ -69,6 +75,7 @@ def initialize(scope, env = nil)

@trace_id ||= Utils.uuid
@span_id = Utils.uuid.slice(0, 16)
@sample_rand ||= generate_sample_rand
end

# Extract the trace_id, parent_span_id and parent_sampled values from a sentry-trace header.
Expand All @@ -77,7 +84,7 @@ def initialize(scope, env = nil)
# @return [Array, nil]
def self.extract_sentry_trace(sentry_trace)
match = SENTRY_TRACE_REGEXP.match(sentry_trace)
return nil if match.nil?
return if match.nil?

trace_id, parent_span_id, sampled_flag = match[1..3]
parent_sampled = sampled_flag.nil? ? nil : sampled_flag != "0"
Expand Down Expand Up @@ -123,6 +130,7 @@ def populate_head_baggage

items = {
"trace_id" => trace_id,
"sample_rand" => Utils::SampleRand.format(@sample_rand),
"environment" => configuration.environment,
"release" => configuration.release,
"public_key" => configuration.dsn&.public_key
Expand All @@ -131,5 +139,32 @@ def populate_head_baggage
items.compact!
@baggage = Baggage.new(items, mutable: false)
end

def extract_sample_rand_from_baggage(baggage)
return unless baggage&.items

sample_rand_str = baggage.items["sample_rand"]
return unless sample_rand_str

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?
generator.generate_from_sampling_decision(@parent_sampled, sample_rate)
else
generator.generate_from_trace_id
end
else
generator.generate_from_trace_id
end
end
end
end
48 changes: 47 additions & 1 deletion sentry-ruby/lib/sentry/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "sentry/baggage"
require "sentry/profiler"
require "sentry/utils/sample_rand"
require "sentry/propagation_context"

module Sentry
Expand Down Expand Up @@ -57,12 +58,17 @@ class Transaction < Span
# @return [Profiler]
attr_reader :profiler

# Sample rand value generated from trace_id
# @return [String]
attr_reader :sample_rand

def initialize(
hub:,
name: nil,
source: :custom,
parent_sampled: nil,
baggage: nil,
sample_rand: nil,
**options
)
super(transaction: self, **options)
Expand All @@ -82,12 +88,18 @@ def initialize(
@effective_sample_rate = nil
@contexts = {}
@measurements = {}
@sample_rand = sample_rand

unless @hub.profiler_running?
@profiler = @configuration.profiler_class.new(@configuration)
end

init_span_recorder

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.
Expand Down Expand Up @@ -123,12 +135,15 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h

baggage.freeze!

sample_rand = extract_sample_rand_from_baggage(baggage, trace_id, parent_sampled)

new(
trace_id: trace_id,
parent_span_id: parent_span_id,
parent_sampled: parent_sampled,
hub: hub,
baggage: baggage,
sample_rand: sample_rand,
**options
)
end
Expand All @@ -139,6 +154,29 @@ def self.extract_sentry_trace(sentry_trace)
PropagationContext.extract_sentry_trace(sentry_trace)
end

def self.extract_sample_rand_from_baggage(baggage, trace_id, parent_sampled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not define these methods again, but add params to the ones in PropagationContext and reuse them?

so expose

  • PropagationContext.extract_sample_rand_from_baggage(baggage)
  • PropgationContext.generate_sample_rand(baggage, trace_id, parent_sampled)

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
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

# @return [Hash]
def to_hash
hash = super
Expand All @@ -153,6 +191,13 @@ def to_hash
hash
end

def parent_sample_rate
return unless @baggage&.items

sample_rate_str = @baggage.items["sample_rate"]
sample_rate_str&.to_f
end

# @return [Transaction]
def deep_dup
copy = super
Expand Down Expand Up @@ -225,7 +270,7 @@ def set_initial_sample_decision(sampling_context:)
@effective_sample_rate /= 2**factor
end

@sampled = Random.rand < @effective_sample_rate
@sampled = @sample_rand < @effective_sample_rate
end

if @sampled
Expand Down Expand Up @@ -331,6 +376,7 @@ def populate_head_baggage
items = {
"trace_id" => trace_id,
"sample_rate" => effective_sample_rate&.to_s,
"sample_rand" => Utils::SampleRand.format(@sample_rand),
"sampled" => sampled&.to_s,
"environment" => @environment,
"release" => @release,
Expand Down
98 changes: 98 additions & 0 deletions sentry-ruby/lib/sentry/utils/sample_rand.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# frozen_string_literal: true

module Sentry
module Utils
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 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
generate_based_on_sampling(sampled, sample_rate)
end
end

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 invalid_sample_rate?(sample_rate)
sample_rate.nil? || sample_rate <= 0.0 || sample_rate > 1.0
end

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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: String Validation Error in SampleRand

The valid? method in SampleRand incorrectly accepts non-numeric string values for sample_rand. These strings convert to 0.0 and pass validation, which can lead to invalid sample_rand values being treated as 0.0. This may cause incorrect sampling decisions, particularly when processing malformed baggage data.

Fix in Cursor Fix in Web


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
end
1 change: 1 addition & 0 deletions sentry-ruby/spec/sentry/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def sentry_context
expect(event.dynamic_sampling_context).to eq({
"environment" => "development",
"public_key" => "12345",
"sample_rand" => Sentry::Utils::SampleRand.format(transaction.sample_rand),
"sample_rate" => "1.0",
"sampled" => "true",
"transaction" => "test transaction",
Expand Down
Loading
Loading