Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sl0thentr0py committed Sep 19, 2022
1 parent a06549f commit c9764dd
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 33 deletions.
2 changes: 0 additions & 2 deletions sentry-ruby/lib/sentry/baggage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ def initialize(sentry_items, third_party_items: '', mutable: true)
# @param header [String] The incoming Baggage header string.
# @return [Baggage, nil]
def self.from_incoming_header(header)
return nil if header.nil? || header.empty?

sentry_items = {}
third_party_items = ''
mutable = true
Expand Down
56 changes: 25 additions & 31 deletions sentry-ruby/lib/sentry/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ def initialize(name: nil, parent_sampled: nil, baggage: nil, hub:, **options)
@traces_sampler = hub.configuration.traces_sampler
@traces_sample_rate = hub.configuration.traces_sample_rate
@logger = hub.configuration.logger
@release = hub.configuration.release
@environment = hub.configuration.environment
@dsn = hub.configuration.dsn
@effective_sample_rate = nil
init_span_recorder
end
Expand Down Expand Up @@ -85,11 +88,15 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h
sampled_flag != "0"
end

# If there's an incoming sentry-trace but no incoming baggage header,
# for instance in traces coming from older SDKs,
# baggage will be empty and frozen and won't be populated as head SDK.
baggage = Baggage.from_incoming_header(baggage) if baggage
baggage ||= Baggage.new({})
baggage = if baggage && !baggage.empty?
Baggage.from_incoming_header(baggage)
else
# If there's an incoming sentry-trace but no incoming baggage header,
# for instance in traces coming from older SDKs,
# baggage will be empty and frozen and won't be populated as head SDK.
Baggage.new({})
end

baggage.freeze!

new(
Expand Down Expand Up @@ -148,18 +155,16 @@ def set_initial_sample_decision(sampling_context:)

transaction_description = generate_transaction_description

unless [true, false].include?(sample_rate) || (sample_rate.is_a?(Numeric) && sample_rate >= 0.0 && sample_rate <= 1.0)
if [true, false].include?(sample_rate)
@effective_sample_rate = sample_rate ? 1.0 : 0.0
elsif sample_rate.is_a?(Numeric) && sample_rate >= 0.0 && sample_rate <= 1.0
@effective_sample_rate = sample_rate.to_f
else
@sampled = false
log_warn("#{MESSAGE_PREFIX} Discarding #{transaction_description} because of invalid sample_rate: #{sample_rate}")
return
end

@effective_sample_rate = if [true, false].include?(sample_rate)
sample_rate ? 1.0 : 0.0
else
sample_rate.to_f
end

if sample_rate == 0.0 || sample_rate == false
@sampled = false
log_debug("#{MESSAGE_PREFIX} Discarding #{transaction_description} because traces_sampler returned 0 or false")
Expand Down Expand Up @@ -235,30 +240,19 @@ def generate_transaction_description
end

def populate_head_baggage
sentry_items = {}
sentry_items["trace_id"] = trace_id

# TODO-neel filter for high cardinality / low quality tx source here
sentry_items["transaction"] = name
sentry_items["sample_rate"] = effective_sample_rate&.to_s

configuration = Sentry.configuration

if configuration
sentry_items["environment"] = configuration.environment
sentry_items["release"] = configuration.release
sentry_items["public_key"] = configuration.dsn&.public_key
end
sentry_items = {
"trace_id" => trace_id,
"transaction" => name,# TODO-neel filter for high cardinality tx source
"sample_rate" => effective_sample_rate&.to_s,
"environment" => @environment,
"release" => @release,
"public_key" => @dsn&.public_key
}

user = Sentry.get_current_scope&.user
sentry_items["user_segment"] = user["segment"] if user && user["segment"]

# there's an existing baggage but it was mutable,
# which is why we are creating this new baggage.
# However, if by chance the user put some sentry items in there, give them precedence.
sentry_items.merge!(@baggage.sentry_items) if @baggage && !@baggage.sentry_items.empty?
sentry_items.compact!

@baggage = Baggage.new(sentry_items, mutable: false)
end

Expand Down
2 changes: 2 additions & 0 deletions sentry-ruby/spec/sentry/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ def sentry_context
event = subject.event_from_transaction(transaction)

expect(event.dynamic_sampling_context).to eq({
"environment" => "development",
"public_key" => "12345",
"sample_rate" => "1.0",
"transaction" => "test transaction",
"trace_id" => transaction.trace_id
Expand Down

0 comments on commit c9764dd

Please sign in to comment.