Skip to content
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

Recording vs sampled #121

Merged
merged 5 commits into from
Oct 16, 2019
Merged
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
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