fix: Set r in Parent CPS (#1455)

open-telemetry · May 29, 2023 · 82869ea · 82869ea
1 parent 1fabd31
commit 82869ea
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 5 deletions.
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
@@ -23,9 +23,14 @@ module ConsistentProbabilityTraceState
           # tracestate sanitized according to the Context invariants defined in the
           # tracestate probability sampling spec.
           #
+          # If r is nil after the sanitization, it is generated from the trace_id.
+          #
+          # This method assumes the parent span context is valid.
+          #
+          # @param trace_id [OpenTelemetry::Trace::TraceId] the trace id
           # @param span_context [OpenTelemetry::Trace::SpanContext] the parent span context
           # @return [OpenTelemetry::Trace::Tracestate] the sanitized tracestate
-          def sanitized_tracestate(span_context)
+          def sanitized_tracestate(trace_id, span_context)
             sampled = span_context.trace_flags.sampled?
             tracestate = span_context.tracestate
             parse_ot_vendor_tag(tracestate) do |p, r, rest|
@@ -35,9 +40,13 @@ def sanitized_tracestate(span_context)
                 p = nil
               elsif !p.nil? && !r.nil? && !invariant(p, r, sampled)
                 p = nil
-              else
+              elsif !r.nil?
                 return tracestate
               end
+              if r.nil?
+                OpenTelemetry.logger.debug("ConsistentProbabilitySampler: potentially inconsistent trace detected - r: #{r.inspect}")
+                r = generate_r(trace_id)
+              end
               update_tracestate(tracestate, p, r, rest)
             end
           end

diff --git a/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/parent_consistent_probability_based.rb b/sdk_experimental/lib/opentelemetry/sdk/trace/samplers/parent_consistent_probability_based.rb
@@ -43,7 +43,7 @@ def should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes:
             if !parent_span_context.valid?
               @root.should_sample?(trace_id: trace_id, parent_context: parent_context, links: links, name: name, kind: kind, attributes: attributes)
             else
-              tracestate = sanitized_tracestate(parent_span_context)
+              tracestate = sanitized_tracestate(trace_id, parent_span_context)
               if parent_span_context.trace_flags.sampled?
                 Result.new(decision: Decision::RECORD_AND_SAMPLE, tracestate: tracestate)
               else

diff --git a/...imental/test/opentelemetry/sdk/trace/samplers/parent_consistent_probability_based_test.rb b/...imental/test/opentelemetry/sdk/trace/samplers/parent_consistent_probability_based_test.rb
@@ -31,8 +31,15 @@
       _(call_sampler(subject, parent_context: parent_context(sampled: false, ot: 'p:2;r:1')).tracestate['ot']).must_equal('p:2;r:1')
       _(call_sampler(subject, parent_context: parent_context(sampled: true, ot: 'p:2;r:1')).tracestate['ot']).must_equal('r:1')
       _(call_sampler(subject, parent_context: parent_context(sampled: true, ot: 'p:63;r:1')).tracestate['ot']).must_equal('p:63;r:1')
-      _(call_sampler(subject, parent_context: parent_context(ot: 'p:1;r:63')).tracestate['ot']).must_be_nil
-      _(call_sampler(subject, parent_context: parent_context(ot: 'p:1;r:63;junk')).tracestate['ot']).must_equal('junk')
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(ot: 'p:1;r:63')).tracestate['ot']).must_equal('r:0')
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(ot: 'p:1;r:63;junk')).tracestate['ot']).must_equal('r:0;junk')
+    end
+
+    it 'sets r based on the trace_id if missing or invalid' do
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(sampled: true)).tracestate['ot']).must_equal('r:0')
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(sampled: false)).tracestate['ot']).must_equal('r:0')
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(ot: 'r:63', sampled: true)).tracestate['ot']).must_equal('r:0')
+      _(call_sampler(subject, trace_id: trace_id(-1), parent_context: parent_context(ot: 'r:63', sampled: false)).tracestate['ot']).must_equal('r:0')
     end
 
     it 'respects parent sampling decision' do