Skip to content

Commit

Permalink
fix! consistent naming for sampling results (#401)
Browse files Browse the repository at this point in the history
* fix! consistent naming for sampling results

* Ignore -> Drop
  • Loading branch information
fbogsany authored Sep 18, 2020
1 parent 302bd41 commit 1c37b00
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 43 deletions.
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

0 comments on commit 1c37b00

Please sign in to comment.