From 77cf7886bb9b5bff41fc72b7ec8476aa88c9faaa Mon Sep 17 00:00:00 2001 From: James Reeve Date: Fri, 3 Jul 2026 14:46:19 -0400 Subject: [PATCH 1/2] fix: build non-recording spans from a real SpanContext Signed-off-by: James Reeve --- lib/instana/instrumentation/action_cable.rb | 8 ++- lib/instana/instrumentation/grpc.rb | 2 +- lib/instana/samplers/samplers.rb | 5 +- lib/instana/trace/tracer.rb | 4 +- test/instrumentation/grpc_test.rb | 23 ++++++++ test/trace/samplers_test.rb | 48 ++++++++++++++++ test/trace/tracer_start_span_test.rb | 61 +++++++++++++++++++++ 7 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 test/trace/samplers_test.rb create mode 100644 test/trace/tracer_start_span_test.rb diff --git a/lib/instana/instrumentation/action_cable.rb b/lib/instana/instrumentation/action_cable.rb index 2cd04d98..1da1b81c 100644 --- a/lib/instana/instrumentation/action_cable.rb +++ b/lib/instana/instrumentation/action_cable.rb @@ -9,7 +9,13 @@ def instana_trace_context end def process - @instana_trace_context ||= ::Instana.tracer.tracing? ? ::Instana.tracer.current_span.context : {} + # Wrapped in non_recording_span by the channel callbacks below, so it + # must be a SpanContext. + @instana_trace_context ||= if ::Instana.tracer.tracing? + ::Instana.tracer.current_span.context + else + OpenTelemetry::Trace::SpanContext::INVALID + end super end end diff --git a/lib/instana/instrumentation/grpc.rb b/lib/instana/instrumentation/grpc.rb index 57dfb4c0..8ceb5ce5 100644 --- a/lib/instana/instrumentation/grpc.rb +++ b/lib/instana/instrumentation/grpc.rb @@ -58,7 +58,7 @@ module GRPCServerInstrumentation kvs = { rpc: {} } metadata = active_call.metadata - incoming_context = {} + incoming_context = nil if metadata.key?('x-instana-t') incoming_context = SpanContext.new(trace_id: ::Instana::Util.header_to_id(metadata['x-instana-t']), span_id: metadata.key?('x-instana-s') ? ::Instana::Util.header_to_id(metadata['x-instana-s']) : nil, diff --git a/lib/instana/samplers/samplers.rb b/lib/instana/samplers/samplers.rb index 1642e7b9..218209ce 100644 --- a/lib/instana/samplers/samplers.rb +++ b/lib/instana/samplers/samplers.rb @@ -68,7 +68,10 @@ def self.trace_id_ratio_based(_) def self.should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes:) # rubocop:disable Metrics/ParameterLists, Lint/UnusedMethodArgument: parent_span_context = OpenTelemetry::Trace.current_span(parent_context).context - tracestate = parent_span_context&.tracestate + # Parents wrapped in non_recording_span can expose a non-SpanContext + # #context; the new span's SpanContext needs a Tracestate, never nil. + tracestate = parent_span_context.tracestate if parent_span_context.respond_to?(:tracestate) + tracestate ||= OpenTelemetry::Trace::Tracestate::DEFAULT Result.new(decision: :__record_only__, tracestate: tracestate) end end diff --git a/lib/instana/trace/tracer.rb b/lib/instana/trace/tracer.rb index 3f7a08f3..326b615c 100644 --- a/lib/instana/trace/tracer.rb +++ b/lib/instana/trace/tracer.rb @@ -323,7 +323,9 @@ def clear! # with_parent=current context, start_timestamp=current time. # def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timestamp: ::Instana::Util.now_in_ms, kind: nil) # rubocop:disable Metrics/ParameterLists - return Instana::Trace.non_recording_span(with_parent) if !::Instana.agent.ready? || !::Instana.config[:tracing][:enabled] + # The placeholder's #context is read back as the parent by the next span + # start, so it must be a SpanContext (INVALID when there is no parent). + return Instana::Trace.non_recording_span(OpenTelemetry::Trace.current_span(with_parent).context) if !::Instana.agent.ready? || !::Instana.config[:tracing][:enabled] with_parent ||= OpenTelemetry::Context.current name ||= 'empty' diff --git a/test/instrumentation/grpc_test.rb b/test/instrumentation/grpc_test.rb index 4c8d31bb..da4fb337 100644 --- a/test/instrumentation/grpc_test.rb +++ b/test/instrumentation/grpc_test.rb @@ -418,6 +418,29 @@ def test_bidi_streamer_failure assert_equal client_span[:p], sdk_span[:s] end + # A call from an untraced client carries no x-instana-t/-s metadata; the + # server still answers normally and records the call as a root span. + def test_server_span_for_request_without_trace_headers + clear_all! + + response = client_stub.ping( + PingPongService::PingRequest.new(message: 'World') + ) + sleep 0.2 + + assert_equal 'Hello World', response.message + + server_span, rest = ::Instana.processor.queued_spans + assert_nil rest + + assert_server_span( + server_span, + call: '/PingPongService/Ping', + call_type: :request_response + ) + assert_nil server_span[:p] + end + def test_no_error_is_raised_and_no_spans_are_created_when_agent_is_not_ready clear_all! error = nil diff --git a/test/trace/samplers_test.rb b/test/trace/samplers_test.rb new file mode 100644 index 00000000..8b262b39 --- /dev/null +++ b/test/trace/samplers_test.rb @@ -0,0 +1,48 @@ +# (c) Copyright IBM Corp. 2026 + +require 'test_helper' +require 'instana/samplers/samplers' + +class SamplersTest < Minitest::Test + # A malformed parent (non_recording_span accepts any object, so a bare + # Context or Hash can surface here) does not crash span creation and still + # yields a real Tracestate, which is copied into the new span's context. + def test_should_sample_tolerates_parents_without_a_real_span_context + [OpenTelemetry::Context.empty, {}].each do |not_a_span_context| + poisoned_span = Instana::Trace.non_recording_span(not_a_span_context) + parent_context = OpenTelemetry::Trace.context_with_span( + poisoned_span, parent_context: OpenTelemetry::Context.empty + ) + + result = Instana::Trace::Samplers.should_sample?( + trace_id: 'trace-id', parent_context: parent_context, + links: nil, name: 'rack', kind: :server, attributes: nil + ) + + assert result.recording? + assert_instance_of OpenTelemetry::Trace::Tracestate, result.tracestate + end + end + + # W3C tracestate received from upstream propagates to child spans + # unchanged. The parent's tracestate differs from the shared default, so a + # substituted default fails the assertion. + def test_should_sample_propagates_the_parent_tracestate + tracestate = OpenTelemetry::Trace::Tracestate.from_string('in=abc;def') + parent_span_context = Instana::Trace::SpanContext.new( + trace_id: 'a' * 32, span_id: 'b' * 16, tracestate: tracestate + ) + parent_context = OpenTelemetry::Trace.context_with_span( + Instana::Trace.non_recording_span(parent_span_context), + parent_context: OpenTelemetry::Context.empty + ) + + result = Instana::Trace::Samplers.should_sample?( + trace_id: 'trace-id', parent_context: parent_context, + links: nil, name: 'rack', kind: :server, attributes: nil + ) + + assert result.recording? + assert_equal 'abc;def', result.tracestate.value('in') + end +end diff --git a/test/trace/tracer_start_span_test.rb b/test/trace/tracer_start_span_test.rb new file mode 100644 index 00000000..0995cb0e --- /dev/null +++ b/test/trace/tracer_start_span_test.rb @@ -0,0 +1,61 @@ +# (c) Copyright IBM Corp. 2026 + +require 'test_helper' + +class TracerStartSpanTest < Minitest::Test + # While the agent is starting up (or tracing is disabled), start_span + # returns a non-recording placeholder span whose #context the next span + # start in the request reads back as its parent. + + # A request arriving with upstream trace headers during agent warmup keeps + # the upstream ids, so spans recorded later join the upstream trace. + def test_start_span_when_agent_not_ready_preserves_the_incoming_span_context + incoming = Instana::Trace::SpanContext.new(trace_id: 'a' * 32, span_id: 'b' * 16) + with_parent = OpenTelemetry::Trace.context_with_span( + Instana::Trace.non_recording_span(incoming), + parent_context: OpenTelemetry::Context.empty + ) + + span = ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack, with_parent: with_parent) + end + + refute_kind_of OpenTelemetry::Context, span.context + assert_equal incoming.trace_id, span.context.trace_id + assert_equal incoming.span_id, span.context.span_id + assert_equal incoming.tracestate, span.context.tracestate + end + + # A library call that starts a span without naming a parent inherits the + # trace already active on the thread. + def test_start_span_when_agent_not_ready_defaults_to_the_current_context + ambient = Instana::Trace::SpanContext.new(trace_id: 'c' * 32, span_id: 'd' * 16) + context = OpenTelemetry::Trace.context_with_span( + Instana::Trace.non_recording_span(ambient), + parent_context: OpenTelemetry::Context.empty + ) + + span = OpenTelemetry::Context.with_current(context) do + ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack) + end + end + + assert_equal ambient.trace_id, span.context.trace_id + assert_equal ambient.span_id, span.context.span_id + end + + # With no incoming trace and none active on the thread, the placeholder + # reports an invalid context, since a fabricated valid trace id would + # become the parent of real spans once the agent comes online, breaking + # those traces. + def test_start_span_when_agent_not_ready_and_no_parent_returns_an_invalid_span_context + span = OpenTelemetry::Context.with_current(OpenTelemetry::Context.empty) do + ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack) + end + end + + refute span.context.valid? + end +end From bca7b3211c9255ba1b0b06dd53fccdce5ee30057 Mon Sep 17 00:00:00 2001 From: James Reeve Date: Mon, 6 Jul 2026 10:26:21 -0400 Subject: [PATCH 2/2] refactor: reorganize tests to align with repo conventions Signed-off-by: James Reeve --- Rakefile | 2 +- .../sampler_test.rb} | 2 +- test/trace/tracer_start_span_test.rb | 61 ------------------- test/trace/tracer_test.rb | 57 +++++++++++++++++ 4 files changed, 59 insertions(+), 63 deletions(-) rename test/{trace/samplers_test.rb => samplers/sampler_test.rb} (98%) delete mode 100644 test/trace/tracer_start_span_test.rb diff --git a/Rakefile b/Rakefile index 176751c5..04c897b7 100644 --- a/Rakefile +++ b/Rakefile @@ -22,7 +22,7 @@ Rake::TestTask.new(:test) do |t| else t.test_files = Dir[ 'test/*_test.rb', - 'test/{agent,trace,backend,snapshot,span_filtering}/*_test.rb' + 'test/{agent,trace,backend,snapshot,span_filtering,samplers}/*_test.rb' ] end end diff --git a/test/trace/samplers_test.rb b/test/samplers/sampler_test.rb similarity index 98% rename from test/trace/samplers_test.rb rename to test/samplers/sampler_test.rb index 8b262b39..627346e3 100644 --- a/test/trace/samplers_test.rb +++ b/test/samplers/sampler_test.rb @@ -3,7 +3,7 @@ require 'test_helper' require 'instana/samplers/samplers' -class SamplersTest < Minitest::Test +class SamplerTest < Minitest::Test # A malformed parent (non_recording_span accepts any object, so a bare # Context or Hash can surface here) does not crash span creation and still # yields a real Tracestate, which is copied into the new span's context. diff --git a/test/trace/tracer_start_span_test.rb b/test/trace/tracer_start_span_test.rb deleted file mode 100644 index 0995cb0e..00000000 --- a/test/trace/tracer_start_span_test.rb +++ /dev/null @@ -1,61 +0,0 @@ -# (c) Copyright IBM Corp. 2026 - -require 'test_helper' - -class TracerStartSpanTest < Minitest::Test - # While the agent is starting up (or tracing is disabled), start_span - # returns a non-recording placeholder span whose #context the next span - # start in the request reads back as its parent. - - # A request arriving with upstream trace headers during agent warmup keeps - # the upstream ids, so spans recorded later join the upstream trace. - def test_start_span_when_agent_not_ready_preserves_the_incoming_span_context - incoming = Instana::Trace::SpanContext.new(trace_id: 'a' * 32, span_id: 'b' * 16) - with_parent = OpenTelemetry::Trace.context_with_span( - Instana::Trace.non_recording_span(incoming), - parent_context: OpenTelemetry::Context.empty - ) - - span = ::Instana.agent.stub(:ready?, false) do - ::Instana.tracer.start_span(:rack, with_parent: with_parent) - end - - refute_kind_of OpenTelemetry::Context, span.context - assert_equal incoming.trace_id, span.context.trace_id - assert_equal incoming.span_id, span.context.span_id - assert_equal incoming.tracestate, span.context.tracestate - end - - # A library call that starts a span without naming a parent inherits the - # trace already active on the thread. - def test_start_span_when_agent_not_ready_defaults_to_the_current_context - ambient = Instana::Trace::SpanContext.new(trace_id: 'c' * 32, span_id: 'd' * 16) - context = OpenTelemetry::Trace.context_with_span( - Instana::Trace.non_recording_span(ambient), - parent_context: OpenTelemetry::Context.empty - ) - - span = OpenTelemetry::Context.with_current(context) do - ::Instana.agent.stub(:ready?, false) do - ::Instana.tracer.start_span(:rack) - end - end - - assert_equal ambient.trace_id, span.context.trace_id - assert_equal ambient.span_id, span.context.span_id - end - - # With no incoming trace and none active on the thread, the placeholder - # reports an invalid context, since a fabricated valid trace id would - # become the parent of real spans once the agent comes online, breaking - # those traces. - def test_start_span_when_agent_not_ready_and_no_parent_returns_an_invalid_span_context - span = OpenTelemetry::Context.with_current(OpenTelemetry::Context.empty) do - ::Instana.agent.stub(:ready?, false) do - ::Instana.tracer.start_span(:rack) - end - end - - refute span.context.valid? - end -end diff --git a/test/trace/tracer_test.rb b/test/trace/tracer_test.rb index 73e2db97..a6448bdd 100644 --- a/test/trace/tracer_test.rb +++ b/test/trace/tracer_test.rb @@ -361,3 +361,60 @@ def test_missing_class_super end end end + +# While the agent is starting up (or tracing is disabled), start_span +# returns a non-recording placeholder span whose #context the next span +# start in the request reads back as its parent. +class TracerStartSpanTest < Minitest::Test + # A request arriving with upstream trace headers during agent warmup keeps + # the upstream ids, so spans recorded later join the upstream trace. + def test_start_span_when_agent_not_ready_preserves_the_incoming_span_context + incoming = Instana::Trace::SpanContext.new(trace_id: 'a' * 32, span_id: 'b' * 16) + with_parent = OpenTelemetry::Trace.context_with_span( + Instana::Trace.non_recording_span(incoming), + parent_context: OpenTelemetry::Context.empty + ) + + span = ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack, with_parent: with_parent) + end + + refute_kind_of OpenTelemetry::Context, span.context + assert_equal incoming.trace_id, span.context.trace_id + assert_equal incoming.span_id, span.context.span_id + assert_equal incoming.tracestate, span.context.tracestate + end + + # A library call that starts a span without naming a parent inherits the + # trace already active on the thread. + def test_start_span_when_agent_not_ready_defaults_to_the_current_context + ambient = Instana::Trace::SpanContext.new(trace_id: 'c' * 32, span_id: 'd' * 16) + context = OpenTelemetry::Trace.context_with_span( + Instana::Trace.non_recording_span(ambient), + parent_context: OpenTelemetry::Context.empty + ) + + span = OpenTelemetry::Context.with_current(context) do + ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack) + end + end + + assert_equal ambient.trace_id, span.context.trace_id + assert_equal ambient.span_id, span.context.span_id + end + + # With no incoming trace and none active on the thread, the placeholder + # reports an invalid context, since a fabricated valid trace id would + # become the parent of real spans once the agent comes online, breaking + # those traces. + def test_start_span_when_agent_not_ready_and_no_parent_returns_an_invalid_span_context + span = OpenTelemetry::Context.with_current(OpenTelemetry::Context.empty) do + ::Instana.agent.stub(:ready?, false) do + ::Instana.tracer.start_span(:rack) + end + end + + refute span.context.valid? + end +end