fix: consistently pass tracestate (#1453)

open-telemetry · May 25, 2023 · 8bfaffa · 8bfaffa
1 parent 1ad4a21
commit 8bfaffa
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 30 deletions.
diff --git a/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/consistent_probability_based.rb b/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/consistent_probability_based.rb
@@ -42,27 +42,16 @@ def ==(other)
           def should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes:)
             parent_span_context = OpenTelemetry::Trace.current_span(parent_context).context
             p = probabilistic_p
-            if parent_span_context.valid?
-              tracestate = parent_span_context.tracestate
-              parse_ot_vendor_tag(tracestate) do |_, in_r, rest|
-                r = if in_r.nil? || in_r > 62
-                      OpenTelemetry.logger.debug("ConsistentProbabilitySampler: potentially inconsistent trace detected - r: #{in_r.inspect}")
-                      generate_r(trace_id)
-                    else
-                      in_r
-                    end
-                if p <= r
-                  Result.new(decision: Decision::RECORD_AND_SAMPLE, tracestate: update_tracestate(tracestate, p, r, rest))
-                else
-                  Result.new(decision: Decision::DROP, tracestate: update_tracestate(tracestate, nil, r, rest))
-                end
+            tracestate = parent_span_context.tracestate
+            parse_ot_vendor_tag(tracestate) do |_, r, rest|
+              if r.nil? || r > 62
+                OpenTelemetry.logger.debug("ConsistentProbabilitySampler: potentially inconsistent trace detected - r: #{r.inspect}") if parent_span_context.valid?
+                r = generate_r(trace_id)
               end
-            else
-              r = generate_r(trace_id)
               if p <= r
-                Result.new(decision: Decision::RECORD_AND_SAMPLE, tracestate: new_tracestate(p: p, r: r))
+                Result.new(decision: Decision::RECORD_AND_SAMPLE, tracestate: update_tracestate(tracestate, p, r, rest))
               else
-                Result.new(decision: Decision::DROP, tracestate: new_tracestate(r: r))
+                Result.new(decision: Decision::DROP, tracestate: update_tracestate(tracestate, nil, r, rest))
               end
             end
           end

diff --git a/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/consistent_probability_tracestate.rb b/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/consistent_probability_tracestate.rb
@@ -88,18 +88,6 @@ def update_tracestate(tracestate, p, r, rest) # rubocop:disable Naming/Uncommuni
             end
           end
 
-          def new_tracestate(p: nil, r: nil) # rubocop:disable Naming/UncommunicativeMethodParamName
-            if p.nil? && r.nil?
-              OpenTelemetry::Trace::Tracestate::DEFAULT
-            elsif p.nil?
-              OpenTelemetry::Trace::Tracestate.from_hash('ot' => "r:#{r}")
-            elsif r.nil?
-              OpenTelemetry::Trace::Tracestate.from_hash('ot' => "p:#{p}")
-            else
-              OpenTelemetry::Trace::Tracestate.from_hash('ot' => "p:#{p};r:#{r}")
-            end
-          end
-
           def invariant(p, r, sampled) # rubocop:disable Naming/UncommunicativeMethodParamName
             ((p <= r) == sampled) || (sampled && (p == 63))
           end

diff --git a/sdk_experimental/test/opentelemetry/sdk/trace/samplers/consistent_probability_based_test.rb b/sdk_experimental/test/opentelemetry/sdk/trace/samplers/consistent_probability_based_test.rb
@@ -28,6 +28,14 @@
       _(result).wont_be :sampled?
     end
 
+    it 'passes through tracestate even if the parent span is invalid' do
+      tracestate = OpenTelemetry::Trace::Tracestate.from_hash({ 'foo' => 'bar' })
+      parent_span_context = OpenTelemetry::Trace::SpanContext.new(tracestate: tracestate)
+      parent_context = OpenTelemetry::Trace.context_with_span(OpenTelemetry::Trace::Span.new(span_context: parent_span_context))
+      result = call_sampler(subject, trace_id: trace_id(1), parent_context: parent_context)
+      _(result.tracestate['foo']).must_equal('bar')
+    end
+
     it 'populates tracestate with the parent r for a sampled child span' do
       tid = trace_id(1)
       ctx = parent_context(trace_id: tid, ot: 'p:1;r:1')