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

fix! consistent naming for sampling results #401

Merged
merged 3 commits into from
Sep 18, 2020
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
18 changes: 9 additions & 9 deletions sdk/lib/opentelemetry/sdk/trace/samplers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ module Trace
# to the {Span} to be created. Can be nil.
# @return [Result] The sampling result.
module Samplers
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_SAMPLED].freeze
RECORD_AND_SAMPLE = Result.new(decision: Decision::RECORD_AND_SAMPLE)
DROP = Result.new(decision: Decision::DROP)
RECORD_ONLY = Result.new(decision: Decision::RECORD_ONLY)
SAMPLING_HINTS = [Decision::DROP, Decision::RECORD_ONLY, Decision::RECORD_AND_SAMPLE].freeze

private_constant(:RECORD_AND_SAMPLED, :NOT_RECORD, :RECORD, :SAMPLING_HINTS)
private_constant(:RECORD_AND_SAMPLE, :DROP, :RECORD_ONLY, :SAMPLING_HINTS)

# Returns a {Result} with {Decision::RECORD_AND_SAMPLED}.
ALWAYS_ON = ConstantSampler.new(result: RECORD_AND_SAMPLED, description: 'AlwaysOnSampler')
# Returns a {Result} with {Decision::RECORD_AND_SAMPLE}.
ALWAYS_ON = ConstantSampler.new(result: RECORD_AND_SAMPLE, description: 'AlwaysOnSampler')

# Returns a {Result} with {Decision::NOT_RECORD}.
ALWAYS_OFF = ConstantSampler.new(result: NOT_RECORD, description: 'AlwaysOffSampler')
# Returns a {Result} with {Decision::DROP}.
ALWAYS_OFF = ConstantSampler.new(result: DROP, description: 'AlwaysOffSampler')

# Returns a new sampler. It delegates to samplers according to the following rules:
#
Expand Down
6 changes: 3 additions & 3 deletions sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ module Samplers
# decision part of a sampling {Result}.
module Decision
# Decision to not record events and not sample.
NOT_RECORD = :__not_record__
DROP = :__drop__

# Decision to record events and not sample.
RECORD = :__record__
RECORD_ONLY = :__record_only__

# Decision to record events and sample.
RECORD_AND_SAMPLED = :__record_and_sampled__
RECORD_AND_SAMPLE = :__record_and_sample__
end
end
end
Expand Down
6 changes: 3 additions & 3 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_SAMPLED].freeze
DECISIONS = [Decision::RECORD_ONLY, Decision::DROP, Decision::RECORD_AND_SAMPLE].freeze
private_constant(:EMPTY_HASH, :DECISIONS)

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

# Returns true if this span should record events, attributes, status, etc.
#
# @return [Boolean] recording decision
def recording?
@decision != Decision::NOT_RECORD
@decision != Decision::DROP
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ def should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes:
# Ignored for sampling decision: parent_context:, links, name, kind, attributes.

if sample?(trace_id)
RECORD_AND_SAMPLED
RECORD_AND_SAMPLE
else
NOT_RECORD
DROP
end
end

Expand Down
40 changes: 20 additions & 20 deletions sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,36 @@

describe '#attributes' do
it 'is empty by default' do
_(Result.new(decision: Decision::RECORD).attributes).must_equal({})
_(Result.new(decision: Decision::RECORD_ONLY).attributes).must_equal({})
end

it 'is an empty hash when initialized with nil' do
_(Result.new(decision: Decision::RECORD, attributes: nil).attributes).must_equal({})
_(Result.new(decision: Decision::RECORD_ONLY, attributes: nil).attributes).must_equal({})
end

it 'reflects values passed in' do
attributes = {
'foo' => 'bar',
'bar' => 'baz'
}
_(Result.new(decision: Decision::RECORD, attributes: attributes).attributes).must_equal(attributes)
_(Result.new(decision: Decision::RECORD_ONLY, attributes: attributes).attributes).must_equal(attributes)
end

it 'returns a frozen hash' do
_(Result.new(decision: Decision::RECORD, attributes: { 'foo' => 'bar' }).attributes).must_be(:frozen?)
_(Result.new(decision: Decision::RECORD_ONLY, attributes: { 'foo' => 'bar' }).attributes).must_be(:frozen?)
end

it 'allows array-valued attributes' do
attributes = { 'foo' => [1, 2, 3] }
_(Result.new(decision: Decision::RECORD, attributes: attributes).attributes).must_equal(attributes)
_(Result.new(decision: Decision::RECORD_ONLY, attributes: attributes).attributes).must_equal(attributes)
end
end

describe '#initialize' do
it 'accepts Decision constants' do
_(Result.new(decision: Decision::RECORD)).wont_be_nil
_(Result.new(decision: Decision::RECORD_AND_SAMPLED)).wont_be_nil
_(Result.new(decision: Decision::NOT_RECORD)).wont_be_nil
_(Result.new(decision: Decision::RECORD_ONLY)).wont_be_nil
_(Result.new(decision: Decision::RECORD_AND_SAMPLE)).wont_be_nil
_(Result.new(decision: Decision::DROP)).wont_be_nil
end

it 'replaces invalid decisions with default' do
Expand All @@ -52,30 +52,30 @@
end

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

it 'returns false when decision is RECORD' do
_(Result.new(decision: Decision::RECORD)).wont_be :sampled?
it 'returns false when decision is RECORD_ONLY' do
_(Result.new(decision: Decision::RECORD_ONLY)).wont_be :sampled?
end

it 'returns false when decision is NOT_RECORD' do
_(Result.new(decision: Decision::NOT_RECORD)).wont_be :sampled?
it 'returns false when decision is DROP' do
_(Result.new(decision: Decision::DROP)).wont_be :sampled?
end
end

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

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

it 'returns false when decision is NOT_RECORD' do
_(Result.new(decision: Decision::NOT_RECORD)).wont_be :recording?
it 'returns false when decision is DROP' do
_(Result.new(decision: Decision::DROP)).wont_be :recording?
end
end
end
2 changes: 1 addition & 1 deletion sdk/test/opentelemetry/sdk/trace/samplers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
describe '.parent_based' do
let(:not_a_sampler) { Minitest::Mock.new }
let(:trace_id) { OpenTelemetry::Trace.generate_trace_id }
let(:result) { Result.new(decision: Decision::RECORD_AND_SAMPLED) }
let(:result) { Result.new(decision: Decision::RECORD_AND_SAMPLE) }
let(:sampled) { OpenTelemetry::Trace::TraceFlags.from_byte(1) }
let(:not_sampled) { OpenTelemetry::Trace::TraceFlags.from_byte(0) }
let(:remote_sampled_parent_context) { OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, remote: true, trace_flags: sampled) }
Expand Down
10 changes: 5 additions & 5 deletions sdk/test/opentelemetry/sdk/trace/tracer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
OpenTelemetry.tracer_provider.tracer('component-tracer', '1.0.0')
end
let(:record_sampler) do
Samplers::ConstantSampler.new(result: Result.new(decision: Decision::RECORD), description: 'RecordSampler')
Samplers::ConstantSampler.new(result: Result.new(decision: Decision::RECORD_ONLY), description: 'RecordSampler')
end

describe '#name' do
Expand Down Expand Up @@ -73,7 +73,7 @@
trace_id = OpenTelemetry::Trace.generate_trace_id
kind = Minitest::Mock.new
attributes = Minitest::Mock.new
result = Result.new(decision: Decision::NOT_RECORD)
result = Result.new(decision: Decision::DROP)
mock_sampler = Minitest::Mock.new
mock_sampler.expect(:should_sample?, result, [{ trace_id: trace_id, parent_context: nil, links: links, name: name, kind: kind, attributes: attributes }])
activate_trace_config TraceConfig.new(sampler: mock_sampler)
Expand Down Expand Up @@ -108,7 +108,7 @@
it 'creates a span with sampler attributes added after supplied attributes' do
sampler_attributes = { '1' => 1 }
mock_sampler = Minitest::Mock.new
result = Result.new(decision: Decision::RECORD, attributes: sampler_attributes)
result = Result.new(decision: Decision::RECORD_ONLY, attributes: sampler_attributes)
mock_sampler.expect(:should_sample?, result, [Hash])
activate_trace_config TraceConfig.new(sampler: mock_sampler)
span = tracer.start_root_span('op', attributes: { '1' => 0, '2' => 2 })
Expand Down Expand Up @@ -204,7 +204,7 @@
name = 'span'
kind = Minitest::Mock.new
attributes = Minitest::Mock.new
result = Result.new(decision: Decision::NOT_RECORD)
result = Result.new(decision: Decision::DROP)
mock_sampler = Minitest::Mock.new
mock_sampler.expect(:should_sample?, result, [{ trace_id: span_context.trace_id, parent_context: span_context, links: links, name: name, kind: kind, attributes: attributes }])
activate_trace_config TraceConfig.new(sampler: mock_sampler)
Expand Down Expand Up @@ -245,7 +245,7 @@
it 'creates a span with sampler attributes added after supplied attributes' do
sampler_attributes = { '1' => 1 }
mock_sampler = Minitest::Mock.new
result = Result.new(decision: Decision::RECORD, attributes: sampler_attributes)
result = Result.new(decision: Decision::RECORD_ONLY, attributes: sampler_attributes)
mock_sampler.expect(:should_sample?, result, [Hash])
activate_trace_config TraceConfig.new(sampler: mock_sampler)
span = tracer.start_span('op', with_parent_context: context, attributes: { '1' => 0, '2' => 2 })
Expand Down