Skip to content

Commit

Permalink
Recording vs sampled (#121)
Browse files Browse the repository at this point in the history
  • Loading branch information
fbogsany authored Oct 16, 2019
1 parent 5337420 commit 3b52ae0
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 49 deletions.
8 changes: 4 additions & 4 deletions api/lib/opentelemetry/trace/sampling_hint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ module Trace
# Hints to influence sampling decisions. The default option for span
# creation is to not provide any suggestion.
module SamplingHint
# Suggest to not record events and not propagate.
# Suggest to not record events and not sample.
NOT_RECORD = :__not_record__

# Suggest to record events and not propagate.
# Suggest to record events and not sample.
RECORD = :__record__

# Suggest to record events and propagate.
RECORD_AND_PROPAGATE = :__record_and_propagate__
# Suggest to record events and sample.
RECORD_AND_SAMPLED = :__record_and_sampled__
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def on_start(span)
end

# adds a span to the batcher, threadsafe may block on lock
def on_finish(span)
return unless span.recording?
def on_finish(span) # rubocop:disable Metrics/AbcSize
return unless span.context.trace_flags.sampled?

lock do
n = spans.size + 1 - max_queue_size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def on_start(span)
#
# @param [Span] span the {Span} that just ended.
def on_finish(span)
return unless span.recording?
return unless span.context.trace_flags.sampled?

@span_exporter.export([span.to_span_data])
rescue => e # rubocop:disable Style/RescueStandardError
Expand Down
14 changes: 7 additions & 7 deletions sdk/lib/opentelemetry/sdk/trace/samplers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,31 @@ module Trace
# to the {Span} to be created. Can be nil.
# @return [Result] The sampling result.
module Samplers
RECORD_AND_PROPAGATE = Result.new(decision: Decision::RECORD_AND_PROPAGATE)
RECORD_AND_SAMPLED = Result.new(decision: Decision::RECORD_AND_SAMPLED)
NOT_RECORD = Result.new(decision: Decision::NOT_RECORD)
RECORD = Result.new(decision: Decision::RECORD)
SAMPLING_HINTS = [Decision::NOT_RECORD, Decision::RECORD, Decision::RECORD_AND_PROPAGATE].freeze
SAMPLING_HINTS = [Decision::NOT_RECORD, Decision::RECORD, Decision::RECORD_AND_SAMPLED].freeze
APPLY_PROBABILITY_TO_SYMBOLS = %i[root_spans root_spans_and_remote_parent all_spans].freeze

private_constant(:RECORD_AND_PROPAGATE, :NOT_RECORD, :RECORD, :SAMPLING_HINTS, :APPLY_PROBABILITY_TO_SYMBOLS)
private_constant(:RECORD_AND_SAMPLED, :NOT_RECORD, :RECORD, :SAMPLING_HINTS, :APPLY_PROBABILITY_TO_SYMBOLS)

# rubocop:disable Lint/UnusedBlockArgument

# Ignores all values in hint and returns a {Result} with
# {Decision::RECORD_AND_PROPAGATE}.
ALWAYS_ON = ->(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attributes:) { RECORD_AND_PROPAGATE }
# {Decision::RECORD_AND_SAMPLED}.
ALWAYS_ON = ->(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attributes:) { RECORD_AND_SAMPLED }

# Ignores all values in hint and returns a {Result} with
# {Decision::NOT_RECORD}.
ALWAYS_OFF = ->(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attributes:) { NOT_RECORD }

# Ignores all values in hint and returns a {Result} with
# {Decision::RECORD_AND_PROPAGATE} if the parent context is sampled or
# {Decision::RECORD_AND_SAMPLED} if the parent context is sampled or
# {Decision::NOT_RECORD} otherwise, or if there is no parent context.
# rubocop:disable Style/Lambda
ALWAYS_PARENT = ->(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attributes:) do
if parent_context&.trace_flags&.sampled?
RECORD_AND_PROPAGATE
RECORD_AND_SAMPLED
else
NOT_RECORD
end
Expand Down
8 changes: 4 additions & 4 deletions sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ module Samplers
# The Decision module contains a set of constants to be used in the
# decision part of a sampling {Result}.
module Decision
# Decision to not record events and not propagate.
# Decision to not record events and not sample.
NOT_RECORD = OpenTelemetry::Trace::SamplingHint::NOT_RECORD

# Decision to record events and not propagate.
# Decision to record events and not sample.
RECORD = OpenTelemetry::Trace::SamplingHint::RECORD

# Decision to record events and propagate.
RECORD_AND_PROPAGATE = OpenTelemetry::Trace::SamplingHint::RECORD_AND_PROPAGATE
# Decision to record events and sample.
RECORD_AND_SAMPLED = OpenTelemetry::Trace::SamplingHint::RECORD_AND_SAMPLED
end
end
end
Expand Down
18 changes: 9 additions & 9 deletions sdk/lib/opentelemetry/sdk/trace/samplers/probability_sampler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ module Samplers
#
# Implements sampling based on a probability.
class ProbabilitySampler
HINT_RECORD_AND_PROPAGATE = OpenTelemetry::Trace::SamplingHint::RECORD_AND_PROPAGATE
HINT_RECORD_AND_SAMPLED = OpenTelemetry::Trace::SamplingHint::RECORD_AND_SAMPLED
HINT_RECORD = OpenTelemetry::Trace::SamplingHint::RECORD

private_constant(:HINT_RECORD_AND_PROPAGATE, :HINT_RECORD)
private_constant(:HINT_RECORD_AND_SAMPLED, :HINT_RECORD)

def initialize(probability, ignore_hints:, ignore_parent:, apply_to_remote_parent:, apply_to_all_spans:)
@probability = probability
Expand All @@ -34,12 +34,12 @@ def call(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attr

hint = nil if @ignored_hints.include?(hint)

sampled_flag = sample?(hint, trace_id, parent_context)
record_events = hint == HINT_RECORD || sampled_flag
sampled = sample?(hint, trace_id, parent_context)
recording = hint == HINT_RECORD || sampled

if sampled_flag && record_events
RECORD_AND_PROPAGATE
elsif record_events
if sampled && recording
RECORD_AND_SAMPLED
elsif recording
RECORD
else
NOT_RECORD
Expand All @@ -50,9 +50,9 @@ def call(trace_id:, span_id:, parent_context:, hint:, links:, name:, kind:, attr

def sample?(hint, trace_id, parent_context)
if parent_context.nil?
hint == HINT_RECORD_AND_PROPAGATE || sample_trace_id?(trace_id)
hint == HINT_RECORD_AND_SAMPLED || sample_trace_id?(trace_id)
else
parent_sampled?(parent_context) || hint == HINT_RECORD_AND_PROPAGATE || sample_trace_id_for_child?(parent_context, trace_id)
parent_sampled?(parent_context) || hint == HINT_RECORD_AND_SAMPLED || sample_trace_id_for_child?(parent_context, trace_id)
end
end

Expand Down
8 changes: 4 additions & 4 deletions sdk/lib/opentelemetry/sdk/trace/samplers/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module Samplers
# root span.
class Result
EMPTY_HASH = {}.freeze
DECISIONS = [Decision::RECORD, Decision::NOT_RECORD, Decision::RECORD_AND_PROPAGATE].freeze
DECISIONS = [Decision::RECORD, Decision::NOT_RECORD, Decision::RECORD_AND_SAMPLED].freeze
private_constant(:EMPTY_HASH, :DECISIONS)

# Returns a frozen hash of attributes to be attached span.
Expand All @@ -40,13 +40,13 @@ def initialize(decision:, attributes: nil)
#
# @return [Boolean] sampling decision
def sampled?
@decision == Decision::RECORD_AND_PROPAGATE
@decision == Decision::RECORD_AND_SAMPLED
end

# Returns true if this span should record events.
# Returns true if this span should record events, attributes, status, etc.
#
# @return [Boolean] recording decision
def record_events?
def recording?
@decision != Decision::NOT_RECORD
end
end
Expand Down
2 changes: 1 addition & 1 deletion sdk/lib/opentelemetry/sdk/trace/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def start_span(name, with_parent: nil, with_parent_context: nil, attributes: nil
private

def internal_create_span(result, name, kind, trace_id, span_id, parent_span_id, attributes, links, start_timestamp)
if result.record_events? && !@stopped
if result.recording? && !@stopped
trace_flags = result.sampled? ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT
context = OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, trace_flags: trace_flags)
attributes = attributes&.merge(result.attributes) || result.attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ def shutdown; end

class TestSpan
def initialize(id = nil, recording = true)
trace_flags = recording ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT
@context = OpenTelemetry::Trace::SpanContext.new(trace_flags: trace_flags)
@id = id
@recording = recording
end

attr_reader :id
attr_reader :id, :context

def recording?
@recording
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
require 'test_helper'

def stub_span_builder(recording: false)
ctx = OpenTelemetry::Trace::SpanContext.new
trace_flags = recording ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT
ctx = OpenTelemetry::Trace::SpanContext.new(trace_flags: trace_flags)
span = OpenTelemetry::Trace::Span.new(span_context: ctx)
def span.to_span_data; end

Expand Down Expand Up @@ -55,13 +56,17 @@ def mock.nil?
mock_span_exporter.verify
end

it 'calls #to_span_data on recorded spans in #on_finish' do
it 'calls #to_span_data on sampled spans in #on_finish' do
processor_noop = export::SimpleSpanProcessor.new(
export::NoopSpanExporter.new
)

mock_trace_flags = Minitest::Mock.new
mock_trace_flags.expect :sampled?, true
mock_span_context = Minitest::Mock.new
mock_span_context.expect :trace_flags, mock_trace_flags
mock_span = Minitest::Mock.new
mock_span.expect :recording?, true
mock_span.expect :context, mock_span_context
mock_span.expect :to_span_data, nil

processor_noop.on_start(mock_span)
Expand Down
16 changes: 8 additions & 8 deletions sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
describe '#initialize' do
it 'accepts Decision constants' do
Result.new(decision: Decision::RECORD).wont_be_nil
Result.new(decision: Decision::RECORD_AND_PROPAGATE).wont_be_nil
Result.new(decision: Decision::RECORD_AND_SAMPLED).wont_be_nil
Result.new(decision: Decision::NOT_RECORD).wont_be_nil
end

Expand All @@ -47,8 +47,8 @@
end

describe '#sampled?' do
it 'returns true when decision is RECORD_AND_PROPAGATE' do
Result.new(decision: Decision::RECORD_AND_PROPAGATE).must_be :sampled?
it 'returns true when decision is RECORD_AND_SAMPLED' do
Result.new(decision: Decision::RECORD_AND_SAMPLED).must_be :sampled?
end

it 'returns false when decision is RECORD' do
Expand All @@ -60,17 +60,17 @@
end
end

describe '#record_events?' do
it 'returns true when decision is RECORD_AND_PROPAGATE' do
Result.new(decision: Decision::RECORD_AND_PROPAGATE).must_be :record_events?
describe '#recording?' do
it 'returns true when decision is RECORD_AND_SAMPLED' do
Result.new(decision: Decision::RECORD_AND_SAMPLED).must_be :recording?
end

it 'returns true when decision is RECORD' do
Result.new(decision: Decision::RECORD).must_be :record_events?
Result.new(decision: Decision::RECORD).must_be :recording?
end

it 'returns false when decision is NOT_RECORD' do
Result.new(decision: Decision::NOT_RECORD).wont_be :record_events?
Result.new(decision: Decision::NOT_RECORD).wont_be :recording?
end
end
end
10 changes: 5 additions & 5 deletions sdk/test/opentelemetry/sdk/trace/samplers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@
sampler = Samplers.probability(0, ignore_hints: nil)
not_record_result = call_sampler(sampler, hint: OpenTelemetry::Trace::SamplingHint::NOT_RECORD)
record_result = call_sampler(sampler, hint: OpenTelemetry::Trace::SamplingHint::RECORD)
record_and_propagate_result = call_sampler(sampler, hint: OpenTelemetry::Trace::SamplingHint::RECORD_AND_PROPAGATE)
record_and_sampled_result = call_sampler(sampler, hint: OpenTelemetry::Trace::SamplingHint::RECORD_AND_SAMPLED)
not_record_result.wont_be :sampled?
not_record_result.wont_be :record_events?
not_record_result.wont_be :recording?
record_result.wont_be :sampled?
record_result.must_be :record_events?
record_and_propagate_result.must_be :sampled?
record_and_propagate_result.must_be :record_events?
record_result.must_be :recording?
record_and_sampled_result.must_be :sampled?
record_and_sampled_result.must_be :recording?
end

it 'does not allow invalid hints in ignore_hints' do
Expand Down

0 comments on commit 3b52ae0

Please sign in to comment.