From 976298d1e796ad02b020a1fe4ed83f3ac11f2376 Mon Sep 17 00:00:00 2001 From: Francis Bogsanyi Date: Fri, 18 Sep 2020 10:20:18 -0400 Subject: [PATCH 1/2] fix! consistent naming for sampling results --- sdk/lib/opentelemetry/sdk/trace/samplers.rb | 18 ++++----- .../sdk/trace/samplers/decision.rb | 6 +-- .../sdk/trace/samplers/result.rb | 6 +-- .../trace/samplers/trace_id_ratio_based.rb | 4 +- .../sdk/trace/samplers/result_test.rb | 40 +++++++++---------- .../opentelemetry/sdk/trace/samplers_test.rb | 2 +- .../opentelemetry/sdk/trace/tracer_test.rb | 10 ++--- 7 files changed, 43 insertions(+), 43 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers.rb b/sdk/lib/opentelemetry/sdk/trace/samplers.rb index ae22e406b..355e45096 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers.rb @@ -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) + IGNORE = Result.new(decision: Decision::IGNORE) + RECORD_ONLY = Result.new(decision: Decision::RECORD_ONLY) + SAMPLING_HINTS = [Decision::IGNORE, Decision::RECORD_ONLY, Decision::RECORD_AND_SAMPLE].freeze - private_constant(:RECORD_AND_SAMPLED, :NOT_RECORD, :RECORD, :SAMPLING_HINTS) + private_constant(:RECORD_AND_SAMPLE, :IGNORE, :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::IGNORE}. + ALWAYS_OFF = ConstantSampler.new(result: IGNORE, description: 'AlwaysOffSampler') # Returns a new sampler. It delegates to samplers according to the following rules: # diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb index 4334160ec..6be70b466 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb @@ -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__ + IGNORE = :__ignore__ # 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 diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb index 8062fd6b2..621c4859d 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb @@ -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::IGNORE, Decision::RECORD_AND_SAMPLE].freeze private_constant(:EMPTY_HASH, :DECISIONS) # Returns a frozen hash of attributes to be attached span. @@ -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::IGNORE end end end diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb index 550d09dea..1185827d6 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb @@ -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 + IGNORE end end diff --git a/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb b/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb index d540dd3ba..7010ba4f5 100644 --- a/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb @@ -12,11 +12,11 @@ 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 @@ -24,24 +24,24 @@ '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::IGNORE)).wont_be_nil end it 'replaces invalid decisions with default' do @@ -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 IGNORE' do + _(Result.new(decision: Decision::IGNORE)).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 IGNORE' do + _(Result.new(decision: Decision::IGNORE)).wont_be :recording? end end end diff --git a/sdk/test/opentelemetry/sdk/trace/samplers_test.rb b/sdk/test/opentelemetry/sdk/trace/samplers_test.rb index 5da6ff063..421dcddcd 100644 --- a/sdk/test/opentelemetry/sdk/trace/samplers_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/samplers_test.rb @@ -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) } diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb index fb7a13340..816958e6e 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb @@ -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 @@ -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::IGNORE) 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) @@ -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 }) @@ -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::IGNORE) 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) @@ -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 }) From 050086cf10c96ac7eeb13ef568968b4af9161e1e Mon Sep 17 00:00:00 2001 From: Francis Bogsanyi Date: Fri, 18 Sep 2020 10:48:51 -0400 Subject: [PATCH 2/2] Ignore -> Drop --- sdk/lib/opentelemetry/sdk/trace/samplers.rb | 10 +++++----- sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb | 2 +- sdk/lib/opentelemetry/sdk/trace/samplers/result.rb | 4 ++-- .../sdk/trace/samplers/trace_id_ratio_based.rb | 2 +- .../opentelemetry/sdk/trace/samplers/result_test.rb | 10 +++++----- sdk/test/opentelemetry/sdk/trace/tracer_test.rb | 4 ++-- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers.rb b/sdk/lib/opentelemetry/sdk/trace/samplers.rb index 355e45096..0c0931ae6 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers.rb @@ -38,17 +38,17 @@ module Trace # @return [Result] The sampling result. module Samplers RECORD_AND_SAMPLE = Result.new(decision: Decision::RECORD_AND_SAMPLE) - IGNORE = Result.new(decision: Decision::IGNORE) + DROP = Result.new(decision: Decision::DROP) RECORD_ONLY = Result.new(decision: Decision::RECORD_ONLY) - SAMPLING_HINTS = [Decision::IGNORE, Decision::RECORD_ONLY, Decision::RECORD_AND_SAMPLE].freeze + SAMPLING_HINTS = [Decision::DROP, Decision::RECORD_ONLY, Decision::RECORD_AND_SAMPLE].freeze - private_constant(:RECORD_AND_SAMPLE, :IGNORE, :RECORD_ONLY, :SAMPLING_HINTS) + private_constant(:RECORD_AND_SAMPLE, :DROP, :RECORD_ONLY, :SAMPLING_HINTS) # Returns a {Result} with {Decision::RECORD_AND_SAMPLE}. ALWAYS_ON = ConstantSampler.new(result: RECORD_AND_SAMPLE, description: 'AlwaysOnSampler') - # Returns a {Result} with {Decision::IGNORE}. - ALWAYS_OFF = ConstantSampler.new(result: IGNORE, 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: # diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb index 6be70b466..10aa635d8 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/decision.rb @@ -12,7 +12,7 @@ module Samplers # decision part of a sampling {Result}. module Decision # Decision to not record events and not sample. - IGNORE = :__ignore__ + DROP = :__drop__ # Decision to record events and not sample. RECORD_ONLY = :__record_only__ diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb index 621c4859d..4d7dcfc91 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb @@ -14,7 +14,7 @@ module Samplers # root span. class Result EMPTY_HASH = {}.freeze - DECISIONS = [Decision::RECORD_ONLY, Decision::IGNORE, Decision::RECORD_AND_SAMPLE].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. @@ -46,7 +46,7 @@ def sampled? # # @return [Boolean] recording decision def recording? - @decision != Decision::IGNORE + @decision != Decision::DROP end end end diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb index 1185827d6..d4e7cefdb 100644 --- a/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb +++ b/sdk/lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb @@ -29,7 +29,7 @@ def should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes: if sample?(trace_id) RECORD_AND_SAMPLE else - IGNORE + DROP end end diff --git a/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb b/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb index 7010ba4f5..2936bbe95 100644 --- a/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/samplers/result_test.rb @@ -41,7 +41,7 @@ it 'accepts Decision constants' do _(Result.new(decision: Decision::RECORD_ONLY)).wont_be_nil _(Result.new(decision: Decision::RECORD_AND_SAMPLE)).wont_be_nil - _(Result.new(decision: Decision::IGNORE)).wont_be_nil + _(Result.new(decision: Decision::DROP)).wont_be_nil end it 'replaces invalid decisions with default' do @@ -60,8 +60,8 @@ _(Result.new(decision: Decision::RECORD_ONLY)).wont_be :sampled? end - it 'returns false when decision is IGNORE' do - _(Result.new(decision: Decision::IGNORE)).wont_be :sampled? + it 'returns false when decision is DROP' do + _(Result.new(decision: Decision::DROP)).wont_be :sampled? end end @@ -74,8 +74,8 @@ _(Result.new(decision: Decision::RECORD_ONLY)).must_be :recording? end - it 'returns false when decision is IGNORE' do - _(Result.new(decision: Decision::IGNORE)).wont_be :recording? + it 'returns false when decision is DROP' do + _(Result.new(decision: Decision::DROP)).wont_be :recording? end end end diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb index 816958e6e..c33bff0cc 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb @@ -73,7 +73,7 @@ trace_id = OpenTelemetry::Trace.generate_trace_id kind = Minitest::Mock.new attributes = Minitest::Mock.new - result = Result.new(decision: Decision::IGNORE) + 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) @@ -204,7 +204,7 @@ name = 'span' kind = Minitest::Mock.new attributes = Minitest::Mock.new - result = Result.new(decision: Decision::IGNORE) + 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)