From 0f3ff3e4dc3fe32c5f00eaed0e8ad23979f662c9 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 18:54:28 -0400 Subject: [PATCH 01/27] sample on all trace and root span tags --- lib/datadog/tracing/trace_operation.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 9ebaf46ef85..2433be929ab 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -161,6 +161,29 @@ def resource @resource || (root_span && root_span.resource) end + # When retrieving tags or metrics we need to include root span tags for sampling purposes + def get_tag(key) + super || (root_span && root_span.get_tag(key)) + end + + + def get_metric(key) + @metric[key?] || (root_span && root_span.get_metric(key)) + end + + def tags + all_tags = {} + all_tags.merge(root_span.tags) if root_span + all_tags.merge(@tags) + all_tags.merge(@metrics) + all_tags + end + + + + @resource || (root_span && root_span.resource) + end + # Returns true if the resource has been explicitly set # # @return [Boolean] From 09b7afdcf8721586c1efcf51953a8d5c9ac9b767 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 20:35:46 -0400 Subject: [PATCH 02/27] sample before digest --- lib/datadog/tracing/distributed/propagation.rb | 1 + lib/datadog/tracing/trace_operation.rb | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/datadog/tracing/distributed/propagation.rb b/lib/datadog/tracing/distributed/propagation.rb index 9ab993eb56e..9ad8b63ee35 100644 --- a/lib/datadog/tracing/distributed/propagation.rb +++ b/lib/datadog/tracing/distributed/propagation.rb @@ -44,6 +44,7 @@ def initialize( # DEV-2.0: if needed. # DEV-2.0: Ideally, we'd have a separate stream to report tracer errors and never # DEV-2.0: touch the active span. + # DEV-3.0: Sample trace here instead of when generating digest. # # @param digest [TraceDigest] # @param data [Hash] diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 9ebaf46ef85..42f68afb305 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -2,7 +2,7 @@ require_relative '../core/environment/identity' require_relative '../core/utils' - +require_relative 'tracer' require_relative 'event' require_relative 'metadata/tagging' require_relative 'sampling/ext' @@ -284,10 +284,14 @@ def flush! # Returns a set of trace headers used for continuing traces. # Used for propagation across execution contexts. # Data should reflect the active state of the trace. + # DEV-3.0: Sampling is a side effect of generating the digest. + # We should move the sample call to inject and right before moving to new contexts(threads, forking etc.) def to_digest # Resolve current span ID span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? + # sample the trace_operation with the tracer + tracer.sample_trace(self) TraceDigest.new( span_id: span_id, From 10af56275e28921eb4d89153c213bc0b5658b984 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 20:43:00 -0400 Subject: [PATCH 03/27] only sample if sampling decision not already made --- lib/datadog/tracing/trace_operation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 42f68afb305..b7097bfede6 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -291,7 +291,7 @@ def to_digest span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? # sample the trace_operation with the tracer - tracer.sample_trace(self) + tracer.sample_trace(self) if @sampled.nil? TraceDigest.new( span_id: span_id, From 5f280bd595fe6f6a28843cb7cbe7711da633f31a Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 20:47:13 -0400 Subject: [PATCH 04/27] linting --- lib/datadog/tracing/trace_operation.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 2433be929ab..3b209c01afa 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -166,7 +166,6 @@ def get_tag(key) super || (root_span && root_span.get_tag(key)) end - def get_metric(key) @metric[key?] || (root_span && root_span.get_metric(key)) end @@ -179,11 +178,6 @@ def tags all_tags end - - - @resource || (root_span && root_span.resource) - end - # Returns true if the resource has been explicitly set # # @return [Boolean] From 7203b5188769a87377cfb58066dc4db8a7972c1e Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 21:04:24 -0400 Subject: [PATCH 05/27] sample trace before flush unless already sampled --- lib/datadog/tracing/tracer.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index a16384ed857..8634ca891da 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -494,7 +494,11 @@ def sample_span(trace_op, span) def flush_trace(trace_op) begin trace = @trace_flush.consume!(trace_op) - write(trace) if trace && !trace.empty? + if trace && !trace.empty? + # check if trace is not sampled + sample_trace(trace) unless trace.sampled? + write(trace) + end rescue StandardError => e FLUSH_TRACE_LOG_ONLY_ONCE.run do Datadog.logger.warn { "Failed to flush trace: #{e.class.name} #{e} at #{Array(e.backtrace).first}" } From 7f4dcebb42c20b39d139dbe7c38b0fc2d191a6d2 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 21:06:57 -0400 Subject: [PATCH 06/27] change conditional --- lib/datadog/tracing/trace_operation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index b7097bfede6..ac820d2ecf1 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -291,7 +291,7 @@ def to_digest span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? # sample the trace_operation with the tracer - tracer.sample_trace(self) if @sampled.nil? + tracer.sample_trace(self) unless sampled? TraceDigest.new( span_id: span_id, From f216d222d7a492a0464518abe5f5366c9ef56618 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 24 Sep 2024 21:09:06 -0400 Subject: [PATCH 07/27] remove sampling at span start --- lib/datadog/tracing/tracer.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index a16384ed857..03b4ecd75cb 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -347,7 +347,6 @@ def bind_trace_events!(trace_op) events.span_before_start.subscribe do |event_span_op, event_trace_op| event_trace_op.service ||= @default_service event_span_op.service ||= @default_service - sample_trace(event_trace_op) if event_span_op && event_span_op.parent_id == 0 end events.span_finished.subscribe do |event_span, event_trace_op| From e9b2cd709f2db4e24534431012f1862f2156fbd4 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 25 Sep 2024 13:19:14 -0400 Subject: [PATCH 08/27] update trace operation to take tracer for sampling --- lib/datadog/tracing/trace_operation.rb | 7 +++++-- lib/datadog/tracing/tracer.rb | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index ac820d2ecf1..833b7954b81 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -75,7 +75,9 @@ def initialize( metrics: nil, trace_state: nil, trace_state_unknown_fields: nil, - remote_parent: false + remote_parent: false, + tracer: nil + ) # Attributes @id = id || Tracing::Utils::TraceId.next_id @@ -98,6 +100,7 @@ def initialize( @profiling_enabled = profiling_enabled @trace_state = trace_state @trace_state_unknown_fields = trace_state_unknown_fields + @tracer = tracer # Generic tags set_tags(tags) if tags @@ -291,7 +294,7 @@ def to_digest span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? # sample the trace_operation with the tracer - tracer.sample_trace(self) unless sampled? + tracer&.sample_trace(self) unless priority_sampled? TraceDigest.new( span_id: span_id, diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index a16384ed857..952074fff8e 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -331,12 +331,14 @@ def build_trace(digest = nil) trace_state: digest.trace_state, trace_state_unknown_fields: digest.trace_state_unknown_fields, remote_parent: digest.span_remote, + tracer: self ) else TraceOperation.new( hostname: hostname, profiling_enabled: profiling_enabled, remote_parent: false, + tracer: self ) end end From 630e6f27fb92fe5f1b80699ad43d1cb80c589a37 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 25 Sep 2024 16:20:18 -0400 Subject: [PATCH 09/27] check sampling priority differently --- lib/datadog/tracing/tracer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index f632e5045f8..8a65167c16a 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -498,7 +498,7 @@ def flush_trace(trace_op) trace = @trace_flush.consume!(trace_op) if trace && !trace.empty? # check if trace is not sampled - sample_trace(trace) unless trace.sampled? + sample_trace(trace) unless trace.priority_sampled? write(trace) end rescue StandardError => e From e6e38f05efc95616e6fc79620a21a752b5d0b89e Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 25 Sep 2024 16:22:24 -0400 Subject: [PATCH 10/27] fix get_metric --- lib/datadog/tracing/trace_operation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 3ed6f559301..12af7e48616 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -170,7 +170,7 @@ def get_tag(key) end def get_metric(key) - @metric[key?] || (root_span && root_span.get_metric(key)) + super || (root_span && root_span.get_metric(key)) end def tags From 83b24f3d4a29d5cdb29ce9e56958c5fc63d4cb53 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Mon, 30 Sep 2024 18:25:04 -0400 Subject: [PATCH 11/27] change to checking sampling_priority before running sample --- lib/datadog/tracing/trace_operation.rb | 2 +- lib/datadog/tracing/tracer.rb | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 12af7e48616..a7471b84470 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -311,7 +311,7 @@ def to_digest span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? # sample the trace_operation with the tracer - tracer&.sample_trace(self) unless priority_sampled? + tracer&.sample_trace(self) unless sampling_priority TraceDigest.new( span_id: span_id, diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index 5ca92df962f..540281250d8 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -493,13 +493,10 @@ def sample_span(trace_op, span) # Flush finished spans from the trace buffer, send them to writer. def flush_trace(trace_op) + sample_trace(trace_op) unless trace_op.sampling_priority begin trace = @trace_flush.consume!(trace_op) - if trace && !trace.empty? - # check if trace is not sampled - sample_trace(trace) unless trace.priority_sampled? - write(trace) - end + write(trace) if trace && !trace.empty? rescue StandardError => e FLUSH_TRACE_LOG_ONLY_ONCE.run do Datadog.logger.warn { "Failed to flush trace: #{e.class.name} #{e} at #{Array(e.backtrace).first}" } From 60b9e09378b29a3a08a9ee99e063a0695a203251 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Mon, 30 Sep 2024 18:25:52 -0400 Subject: [PATCH 12/27] integration tests for sampling with tag and resource combinations --- spec/datadog/tracing/integration_spec.rb | 87 +++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/spec/datadog/tracing/integration_spec.rb b/spec/datadog/tracing/integration_spec.rb index 4aa9cd4fe66..7420dd5b2ab 100644 --- a/spec/datadog/tracing/integration_spec.rb +++ b/spec/datadog/tracing/integration_spec.rb @@ -222,7 +222,11 @@ def agent_receives_span_step3(previous_success) @span = trace.spans[0] end - tracer.trace('my.op').finish + tracer.trace('my.op', service: 'my.service') do |span| + span.set_tag('tag', 'tag_value') + span.set_tag('tag2', 'tag_value2') + span.resource = 'my.resource' + end try_wait_until { tracer.writer.stats[:traces_flushed] >= 1 } @@ -319,6 +323,87 @@ def agent_receives_span_step3(previous_success) it_behaves_like 'sampling decision', '-3' end + context 'with a matching resource name' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) { { resource: 'my.resource', sample_rate: 1.0 } } + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP + it_behaves_like 'rule sampling rate metric', 1.0 + it_behaves_like 'rate limit metric', 1.0 + it_behaves_like 'sampling decision', '-3' + end + + context 'with a matching service name' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) { { service: 'my.service', sample_rate: 1.0 } } + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP + it_behaves_like 'rule sampling rate metric', 1.0 + it_behaves_like 'rate limit metric', 1.0 + it_behaves_like 'sampling decision', '-3' + end + + context 'with matching tags' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) { { tags: { tag: 'tag_value', tag2: 'tag_value2' }, sample_rate: 1.0 } } + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP + it_behaves_like 'rule sampling rate metric', 1.0 + it_behaves_like 'rate limit metric', 1.0 + it_behaves_like 'sampling decision', '-3' + end + + context 'with matching tags and matching service and matching resource' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) do + { resource: 'my.resource', service: 'my.service', tags: { tag: 'tag_value', tag2: 'tag_value2' }, + sample_rate: 1.0 } + end + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP + it_behaves_like 'rule sampling rate metric', 1.0 + it_behaves_like 'rate limit metric', 1.0 + it_behaves_like 'sampling decision', '-3' + end + + context 'with not matching tags and matching service and matching resource' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) do + { resource: 'my.resource', service: 'my.service', tags: { tag: 'wrong_tag_value' }, + sample_rate: 1.0 } + end + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::AUTO_KEEP + it_behaves_like 'rule sampling rate metric', nil # Rule is not applied + it_behaves_like 'rate limit metric', nil # Rate limiter is never reached, thus has no value to provide + it_behaves_like 'sampling decision', '-0' + end + + context 'drop with matching tags and matching service and matching resource' do + include_context 'DD_TRACE_SAMPLING_RULES configuration' do + let(:rule) do + { resource: 'my.resource', service: 'my.service', tags: { tag: 'tag_value' }, + sample_rate: 0 } + end + end + + it_behaves_like 'flushed trace' + it_behaves_like 'priority sampled', Datadog::Tracing::Sampling::Ext::Priority::USER_REJECT + it_behaves_like 'rule sampling rate metric', 0.0 + it_behaves_like 'rate limit metric', nil # Rate limiter is never reached, thus has no value to provide + it_behaves_like 'sampling decision', nil + end + context 'with low sample rate' do let(:rule) { Datadog::Tracing::Sampling::SimpleRule.new(sample_rate: Float::MIN) } From fac3d5d671186abe79f66563ece50319e665a1e2 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 2 Oct 2024 18:07:32 -0400 Subject: [PATCH 13/27] broken trace_operation_spec test --- spec/datadog/tracing/trace_operation_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index 64e4de4ac26..d82d7e8aaad 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -136,6 +136,22 @@ end context 'given' do + context ':trace_operation_samples' do + let(:tracer) { instance_double(Datadog::Tracing::Tracer) } + let(:trace_op) { described_class.new(tracer: tracer) } + + describe '#to_digest' do + before do + allow(tracer).to receive(:sample_trace) + end + + it 'calls tracer.sample_trace' do + expect(tracer).to receive(:sample_trace).with(trace_op) + trace_op.to_digest + end + end + end + context ':agent_sample_rate' do subject(:options) { { agent_sample_rate: agent_sample_rate } } let(:agent_sample_rate) { 0.5 } From fab63c7701822b472338e92b817373fe24e3df9d Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 2 Oct 2024 18:58:50 -0400 Subject: [PATCH 14/27] fix how we call tracer in trace operation --- lib/datadog/tracing/trace_operation.rb | 2 +- lib/datadog/tracing/tracer.rb | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index a7471b84470..6ae40f025a9 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -311,7 +311,7 @@ def to_digest span_id = @active_span && @active_span.id span_id ||= @parent_span_id unless finished? # sample the trace_operation with the tracer - tracer&.sample_trace(self) unless sampling_priority + @tracer&.sample_trace(self) unless sampling_priority TraceDigest.new( span_id: span_id, diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index 540281250d8..c5013d553e4 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -262,6 +262,17 @@ def continue_trace!(digest, key = nil, &block) context.activate!(trace, &block) end + # Sample a span, tagging the trace as appropriate. + def sample_trace(trace_op) + begin + @sampler.sample!(trace_op) + rescue StandardError => e + SAMPLE_TRACE_LOG_ONLY_ONCE.run do + Datadog.logger.warn { "Failed to sample trace: #{e.class.name} #{e} at #{Array(e.backtrace).first}" } + end + end + end + # @!visibility private # TODO: make this private def trace_completed @@ -464,17 +475,6 @@ def subscribe_trace_deactivation!(context, trace, original_trace) end end - # Sample a span, tagging the trace as appropriate. - def sample_trace(trace_op) - begin - @sampler.sample!(trace_op) - rescue StandardError => e - SAMPLE_TRACE_LOG_ONLY_ONCE.run do - Datadog.logger.warn { "Failed to sample trace: #{e.class.name} #{e} at #{Array(e.backtrace).first}" } - end - end - end - SAMPLE_TRACE_LOG_ONLY_ONCE = Core::Utils::OnlyOnce.new private_constant :SAMPLE_TRACE_LOG_ONLY_ONCE From bba15ec028cddc96dfea2b308bf98464375ab9c3 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 14:49:11 -0400 Subject: [PATCH 15/27] handle non-integer float edge case --- lib/datadog/tracing/sampling/matcher.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/datadog/tracing/sampling/matcher.rb b/lib/datadog/tracing/sampling/matcher.rb index 4bca551c40e..e3940977108 100644 --- a/lib/datadog/tracing/sampling/matcher.rb +++ b/lib/datadog/tracing/sampling/matcher.rb @@ -44,6 +44,10 @@ def self.glob_to_regex(glob) Regexp.new(glob, Regexp::IGNORECASE) end + def match_all? + pattern == MATCH_ALL_PATTERN + end + # Returns `true` for any input MATCH_ALL = Class.new do def match?(_other) @@ -99,6 +103,10 @@ def match?(trace) def tags_match?(trace) @tags.all? do |name, matcher| tag = trace.get_tag(name) + # Floats: Matching floating point values with a non-zero decimal part is not supported. + # For floating point values with a non-zero decimal part, any all * pattern always returns true. + # Other patterns always return false. + return false if tag.is_a?(Float) && !tag.integer? && !matcher.match_all? # Format metrics as strings, to allow for partial number matching (/4.*/ matching '400', '404', etc.). # Because metrics are floats, we use the '%g' format specifier to avoid trailing zeros, which From 5f4c58a5f72020de1d22ad338eee38b5103d6ed3 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 17:20:26 -0400 Subject: [PATCH 16/27] handle non-integer float edge case --- lib/datadog/tracing/sampling/matcher.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/tracing/sampling/matcher.rb b/lib/datadog/tracing/sampling/matcher.rb index e3940977108..84645c3c32d 100644 --- a/lib/datadog/tracing/sampling/matcher.rb +++ b/lib/datadog/tracing/sampling/matcher.rb @@ -44,7 +44,7 @@ def self.glob_to_regex(glob) Regexp.new(glob, Regexp::IGNORECASE) end - def match_all? + def self.match_all?(pattern) pattern == MATCH_ALL_PATTERN end @@ -106,7 +106,7 @@ def tags_match?(trace) # Floats: Matching floating point values with a non-zero decimal part is not supported. # For floating point values with a non-zero decimal part, any all * pattern always returns true. # Other patterns always return false. - return false if tag.is_a?(Float) && !tag.integer? && !matcher.match_all? + return false if tag.is_a?(Float) && tag.truncate != tag && matcher != MATCH_ALL # Format metrics as strings, to allow for partial number matching (/4.*/ matching '400', '404', etc.). # Because metrics are floats, we use the '%g' format specifier to avoid trailing zeros, which From 9b3729ddddbec21a1c656c507f13c478b3c44fde Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 17:29:39 -0400 Subject: [PATCH 17/27] fix tracer_spec tests since sampling happens at end --- spec/datadog/tracing/tracer_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/datadog/tracing/tracer_spec.rb b/spec/datadog/tracing/tracer_spec.rb index cbed78e91f5..fe280af79a4 100644 --- a/spec/datadog/tracing/tracer_spec.rb +++ b/spec/datadog/tracing/tracer_spec.rb @@ -750,7 +750,7 @@ tracer.trace('operation') do |span, trace| expect(trace).to have_attributes( origin: nil, - sampling_priority: 1 + sampling_priority: nil ) expect(span).to have_attributes( @@ -793,7 +793,7 @@ tracer.trace('operation') do |span, trace| expect(trace).to have_attributes( origin: nil, - sampling_priority: 1 + sampling_priority: nil ) expect(span).to have_attributes( @@ -874,7 +874,7 @@ tracer.trace('second') do |span, trace| expect(trace).to have_attributes( origin: nil, - sampling_priority: 1 + sampling_priority: nil ) expect(span.trace_id).to_not eq(digest.trace_id) @@ -914,7 +914,7 @@ tracer.trace('operation') do |span, trace| expect(trace).to have_attributes( origin: nil, - sampling_priority: 1 + sampling_priority: nil ) expect(span).to have_attributes( From 424b9b5a0a2732b2bd479585a604179e7603fc3a Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 17:43:18 -0400 Subject: [PATCH 18/27] add edge case test for non zero decimal matching --- spec/datadog/tracing/sampling/matcher_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/datadog/tracing/sampling/matcher_spec.rb b/spec/datadog/tracing/sampling/matcher_spec.rb index d50de571cc9..8f81cf98548 100644 --- a/spec/datadog/tracing/sampling/matcher_spec.rb +++ b/spec/datadog/tracing/sampling/matcher_spec.rb @@ -120,6 +120,13 @@ let(:tags) { { 'metric1' => '1', 'metric2' => '3' } } it { is_expected.to eq(false) } + + context 'with a float that has a non-zero decimal' do + let(:tags) { { 'metric1' => '2*' } } + let(:trace_tags) { { 'metric1' => 20.1 } } + + it { is_expected.to eq(false) } + end end end From 9390ccce51fdd3ae161a6b2344af2d6c8803d891 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 17:49:11 -0400 Subject: [PATCH 19/27] test --- lib/datadog/tracing/sampling/matcher.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/datadog/tracing/sampling/matcher.rb b/lib/datadog/tracing/sampling/matcher.rb index 84645c3c32d..658b789552f 100644 --- a/lib/datadog/tracing/sampling/matcher.rb +++ b/lib/datadog/tracing/sampling/matcher.rb @@ -103,6 +103,11 @@ def match?(trace) def tags_match?(trace) @tags.all? do |name, matcher| tag = trace.get_tag(name) + puts "tag: #{tag}" + puts "matcher: #{matcher}" + puts 'hello' + puts matcher != MATCH_ALL + # Floats: Matching floating point values with a non-zero decimal part is not supported. # For floating point values with a non-zero decimal part, any all * pattern always returns true. # Other patterns always return false. From 140355a6e1aa91e6f9e0bdfef5a2b4ebedf6ce1b Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 18:09:02 -0400 Subject: [PATCH 20/27] it works --- lib/datadog/tracing/sampling/matcher.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/datadog/tracing/sampling/matcher.rb b/lib/datadog/tracing/sampling/matcher.rb index 658b789552f..e66cf892914 100644 --- a/lib/datadog/tracing/sampling/matcher.rb +++ b/lib/datadog/tracing/sampling/matcher.rb @@ -103,10 +103,6 @@ def match?(trace) def tags_match?(trace) @tags.all? do |name, matcher| tag = trace.get_tag(name) - puts "tag: #{tag}" - puts "matcher: #{matcher}" - puts 'hello' - puts matcher != MATCH_ALL # Floats: Matching floating point values with a non-zero decimal part is not supported. # For floating point values with a non-zero decimal part, any all * pattern always returns true. From 7c483e44b6908724126f3bc8fc8c4f9bbf487ee7 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 3 Oct 2024 18:23:17 -0400 Subject: [PATCH 21/27] * is the same as infinite * --- lib/datadog/tracing/sampling/matcher.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/datadog/tracing/sampling/matcher.rb b/lib/datadog/tracing/sampling/matcher.rb index e66cf892914..27347322200 100644 --- a/lib/datadog/tracing/sampling/matcher.rb +++ b/lib/datadog/tracing/sampling/matcher.rb @@ -28,7 +28,7 @@ def match?(trace) # @return [#match?(String)] def self.glob_to_regex(glob) # Optimization for match-all case - return MATCH_ALL if glob == MATCH_ALL_PATTERN + return MATCH_ALL if /\A\*+\z/.match?(glob) # Ensure no undesired characters are treated as regex. glob = Regexp.quote(glob) @@ -44,10 +44,6 @@ def self.glob_to_regex(glob) Regexp.new(glob, Regexp::IGNORECASE) end - def self.match_all?(pattern) - pattern == MATCH_ALL_PATTERN - end - # Returns `true` for any input MATCH_ALL = Class.new do def match?(_other) From edf5c2f287886df49a49960ef669f086522b1699 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 4 Oct 2024 13:04:24 -0400 Subject: [PATCH 22/27] * is the same as infinite * edge case test --- spec/datadog/tracing/sampling/matcher_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/datadog/tracing/sampling/matcher_spec.rb b/spec/datadog/tracing/sampling/matcher_spec.rb index 8f81cf98548..b5165a709cd 100644 --- a/spec/datadog/tracing/sampling/matcher_spec.rb +++ b/spec/datadog/tracing/sampling/matcher_spec.rb @@ -158,6 +158,13 @@ it { is_expected.to eq(true) } end + + context 'when multiple *s are used' do + let(:trace_service) { 'hello_service' } + let(:service) { '***' } + + it { is_expected.to eq(true) } + end end context 'with a resource matcher' do From 34457d44dc7c30cdb4f650f6b6ff1cb41f62409d Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 4 Oct 2024 13:22:38 -0400 Subject: [PATCH 23/27] another test for matching float w/ zero decimal --- spec/datadog/tracing/sampling/matcher_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/datadog/tracing/sampling/matcher_spec.rb b/spec/datadog/tracing/sampling/matcher_spec.rb index b5165a709cd..e1f28b63363 100644 --- a/spec/datadog/tracing/sampling/matcher_spec.rb +++ b/spec/datadog/tracing/sampling/matcher_spec.rb @@ -127,6 +127,13 @@ it { is_expected.to eq(false) } end + + context 'with a float that has a zero decimal' do + let(:tags) { { 'metric1' => '2*' } } + let(:trace_tags) { { 'metric1' => 20.0 } } + + it { is_expected.to eq(true) } + end end end From bd41d98b651a767d450de78723b31db8bd7f2b3a Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 4 Oct 2024 20:11:09 -0400 Subject: [PATCH 24/27] tags test broken --- lib/datadog/tracing/trace_operation.rb | 6 +++--- spec/datadog/tracing/trace_operation_spec.rb | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 6ae40f025a9..cd28ed9c0c4 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -175,9 +175,9 @@ def get_metric(key) def tags all_tags = {} - all_tags.merge(root_span.tags) if root_span - all_tags.merge(@tags) - all_tags.merge(@metrics) + all_tags.merge!(root_span&.tags || {}) if root_span + all_tags.merge!(@tags || {}) + all_tags.merge!(@metrics || {}) all_tags end diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index d82d7e8aaad..bfccc10e352 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -317,6 +317,20 @@ end end + context 'when trace operation returns root span values as well' do + let(:options) { { tags: { ok: 'test' } } } + context 'for tags' do + it do + # When tags are added to the root span they should be accessible through the trace operation + span = trace_op.build_span('test', tags: { 'foo' => 'bar' }) + span.start + expect(trace_op.get_tag('foo')).to eq('bar') + expect(trace_op.get_tag('ok')).to eq('test') + expect(trace_op.tags).to eq('foo' => 'bar', 'ok' => 'test') + end + end + end + context 'when :max_length is non-zero' do let(:options) { { max_length: 3 } } From 469a92bb2edee7277d1ef42eda88f64553247332 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 4 Oct 2024 20:23:54 -0400 Subject: [PATCH 25/27] fix tags test if meta is allowed in tags --- lib/datadog/tracing/trace_operation.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index cd28ed9c0c4..0299c2aa3a3 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -178,6 +178,7 @@ def tags all_tags.merge!(root_span&.tags || {}) if root_span all_tags.merge!(@tags || {}) all_tags.merge!(@metrics || {}) + all_tags.merge!(@meta || {}) all_tags end From 5648a4f47413c88d96897417558b27e2c66c3d68 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Mon, 7 Oct 2024 11:10:03 -0400 Subject: [PATCH 26/27] trace operation tag takes precedence over root span tag --- lib/datadog/tracing/trace_operation.rb | 5 ++--- spec/datadog/tracing/trace_operation_spec.rb | 9 +++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 0299c2aa3a3..eb85182af75 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -176,9 +176,8 @@ def get_metric(key) def tags all_tags = {} all_tags.merge!(root_span&.tags || {}) if root_span - all_tags.merge!(@tags || {}) - all_tags.merge!(@metrics || {}) - all_tags.merge!(@meta || {}) + all_tags.merge!(super) + all_tags end diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index bfccc10e352..95b26f3fee4 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -328,6 +328,15 @@ expect(trace_op.get_tag('ok')).to eq('test') expect(trace_op.tags).to eq('foo' => 'bar', 'ok' => 'test') end + + context 'trace operation tags take precedent over root span tags' do + it do + # When tags are added to the root span they should be accessible through the trace operation + span = trace_op.build_span('test', tags: { 'ok' => 'should_not_be' }) + span.start + expect(trace_op.tags).to eq('ok' => 'test') + end + end end end From f5f824be8646f3342470d5dd4c0d838a0dee9ce1 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Mon, 7 Oct 2024 11:22:06 -0400 Subject: [PATCH 27/27] test metrics --- spec/datadog/tracing/trace_operation_spec.rb | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index 95b26f3fee4..d168a99f73c 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -327,6 +327,7 @@ expect(trace_op.get_tag('foo')).to eq('bar') expect(trace_op.get_tag('ok')).to eq('test') expect(trace_op.tags).to eq('foo' => 'bar', 'ok' => 'test') + span.finish end context 'trace operation tags take precedent over root span tags' do @@ -335,6 +336,33 @@ span = trace_op.build_span('test', tags: { 'ok' => 'should_not_be' }) span.start expect(trace_op.tags).to eq('ok' => 'test') + span.finish + end + + context 'for metrics' do + let(:options) { { metrics: { metric1: 123 } } } + it do + # When tags are added to the root span they should be accessible through the trace operation + span = trace_op.build_span('test', tags: { 'metric2' => 456 }) + span.start + expect(trace_op.get_metric('metric1')).to eq(123) + expect(trace_op.get_metric('metric2')).to eq(456) + + span.finish + end + end + + context 'for metrics override' do + let(:options) { { metrics: { metric1: 123 } } } + + it do + # When tags are added to the root span they should be accessible through the trace operation + span = trace_op.build_span('test', tags: { 'metric1' => 456 }) + span.start + expect(trace_op.get_metric('metric1')).to eq(123) + expect(trace_op.tags).to eq({ 'metric1' => 123 }) + span.finish + end end end end