Verified Commit ba58737b authored by Hercules Merscher's avatar Hercules Merscher 🌴
Browse files

fix: OTel span vs context

parent 44634a96
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ module Labkit
          def call(_worker_class, job, _queue, _redis_pool)
            Labkit::Tracing::TracingUtils.with_tracing(operation_name: "sidekiq:#{job_class(job)}", tags: tags_from_job(job, SPAN_KIND)) do |span|
              # Inject the details directly into the job
              Labkit::Tracing::TracingUtils.tracer.inject_context(span.context, job)
              Labkit::Tracing::TracingUtils.tracer.inject_context(span, job)

              yield
            end
+16 −13
Original line number Diff line number Diff line
# frozen_string_literal: true

require "opentracing"

module Labkit
  module Tracing
    module Adapters
@@ -46,11 +44,15 @@ module Labkit
          OpenTelemetry.propagation.extract(carrier)
        end

        def inject_context(span_context, carrier, format: nil) # rubocop:disable Lint/UnusedMethodArgument
        def inject_context(span_wrapper, carrier, format: nil) # rubocop:disable Lint/UnusedMethodArgument
          # Format parameter is ignored for OpenTelemetry - propagation format is configured
          # globally via OTEL_PROPAGATORS environment variable, not per-call like OpenTracing.
          # The parameter exists only for BaseTracer API compatibility.
          context = OpenTelemetry::Trace.context_with_span(span_context)
          #
          # span_wrapper is expected to be a BaseSpan (e.g., OpentelemetrySpan) that wraps
          # the actual OpenTelemetry span. We unwrap it to get the OTel span object.
          span = span_wrapper.respond_to?(:span) ? span_wrapper.span : span_wrapper
          context = OpenTelemetry::Trace.context_with_span(span)
          OpenTelemetry.propagation.inject(carrier, context: context)
        end

@@ -60,24 +62,25 @@ module Labkit

        def start_active_span(operation_name, tags: nil)
          attributes = tags || {}
          span = tracer.start_span(operation_name, attributes: attributes)
          ctx = OpenTelemetry::Trace.context_with_span(span)
          OpenTelemetry::Context.attach(ctx)
          raw_span = tracer.start_span(operation_name, attributes: attributes)
          ctx = OpenTelemetry::Trace.context_with_span(raw_span)
          token = OpenTelemetry::Context.attach(ctx)

          OpenTelemetryScope.new(span, ctx)
          OpenTelemetryScope.new(raw_span, token)
        end

        class OpenTelemetryScope
          attr_reader :span

          def initialize(span, context)
            @span = span
            @context = context
          def initialize(raw_span, token)
            @span = OpentelemetrySpan.new(raw_span)
            @raw_span = raw_span
            @token = token
          end

          def close
            @span.finish
            OpenTelemetry::Context.detach(@context)
            @raw_span.finish
            OpenTelemetry::Context.detach(@token)
          end
        end
      end
+1 −1
Original line number Diff line number Diff line
@@ -36,7 +36,7 @@ module Labkit
          tags = { "component" => "grpc", "span.kind" => "client", "grpc.method" => method, "grpc.type" => grpc_type }

          TracingUtils.with_tracing(operation_name: "grpc:#{method}", tags: tags) do |span|
            TracingUtils.tracer.inject_context(span.context, metadata)
            TracingUtils.tracer.inject_context(span, metadata)

            yield
          end
+30 −8
Original line number Diff line number Diff line
# frozen_string_literal: true

require "opentelemetry/sdk"
require "opentracing"
require "spec_helper"

describe Labkit::Tracing::Adapters::OpentelemetryTracer do
@@ -31,7 +32,8 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      scope = adapter.start_active_span("test_op")

      expect(scope).to be_a(Labkit::Tracing::Adapters::OpentelemetryTracer::OpenTelemetryScope)
      expect(scope.span).to eq(fake_span)
      expect(scope.span).to be_a(Labkit::Tracing::Adapters::OpentelemetrySpan)
      expect(scope.span.span).to eq(fake_span)
    end

    it "accepts tags as attributes" do
@@ -39,7 +41,8 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      expect(fake_tracer).to receive(:start_span).with("test_op", attributes: tags).and_return(fake_span)

      scope = adapter.start_active_span("test_op", tags: tags)
      expect(scope.span).to eq(fake_span)
      expect(scope.span).to be_a(Labkit::Tracing::Adapters::OpentelemetrySpan)
      expect(scope.span.span).to eq(fake_span)
    end

    it "attaches the context" do
@@ -60,10 +63,19 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      it "detaches the context when closed" do
        scope = adapter.start_active_span("test_op")

        expect(OpenTelemetry::Context).to receive(:detach)
        expect(OpenTelemetry::Context).to receive(:detach).with(anything)

        scope.close
      end

      it "passes the attach token to detach" do
        fake_token = instance_double(Object)
        expect(OpenTelemetry::Context).to receive(:attach).with(fake_context).and_return(fake_token)
        scope = adapter.start_active_span("test_op")

        expect(OpenTelemetry::Context).to receive(:detach).with(fake_token)
        scope.close
      end
    end
  end

@@ -102,6 +114,7 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
  describe "#inject_context" do
    let(:propagation) { instance_double(OpenTelemetry::Context::Propagation::CompositeTextMapPropagator) }
    let(:context_with_span) { OpenTelemetry::Context.current }
    let(:wrapped_span) { Labkit::Tracing::Adapters::OpentelemetrySpan.new(fake_span) }

    before do
      allow(OpenTelemetry).to receive(:propagation).and_return(propagation)
@@ -113,7 +126,7 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      expect(OpenTelemetry::Trace).to receive(:context_with_span).with(fake_span)
      expect(propagation).to receive(:inject).with(carrier, context: context_with_span)

      adapter.inject_context(fake_span, carrier)
      adapter.inject_context(wrapped_span, carrier)
    end

    it "injects context into different carrier types" do
@@ -121,19 +134,20 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      expect(OpenTelemetry::Trace).to receive(:context_with_span).with(fake_span)
      expect(propagation).to receive(:inject).with(metadata_carrier, context: context_with_span)

      adapter.inject_context(fake_span, metadata_carrier)
      adapter.inject_context(wrapped_span, metadata_carrier)
    end

    it "uses the provided span_context not current context" do
    it "uses the provided span not current context" do
      carrier = {}
      different_span = instance_double(OpenTelemetry::SDK::Trace::Span)
      different_wrapped_span = Labkit::Tracing::Adapters::OpentelemetrySpan.new(different_span)
      different_context = OpenTelemetry::Context.current
      allow(OpenTelemetry::Trace).to receive(:context_with_span).with(different_span).and_return(different_context)

      expect(OpenTelemetry::Trace).to receive(:context_with_span).with(different_span)
      expect(propagation).to receive(:inject).with(carrier, context: different_context)

      adapter.inject_context(different_span, carrier)
      adapter.inject_context(different_wrapped_span, carrier)
    end

    it "ignores format parameter for backward compatibility" do
@@ -141,7 +155,15 @@ describe Labkit::Tracing::Adapters::OpentelemetryTracer do
      expect(OpenTelemetry::Trace).to receive(:context_with_span).with(fake_span)
      expect(propagation).to receive(:inject).with(carrier, context: context_with_span)

      adapter.inject_context(fake_span, carrier, format: OpenTracing::FORMAT_TEXT_MAP)
      adapter.inject_context(wrapped_span, carrier, format: OpenTracing::FORMAT_TEXT_MAP)
    end

    it "handles unwrapped spans for compatibility" do
      carrier = {}
      expect(OpenTelemetry::Trace).to receive(:context_with_span).with(fake_span)
      expect(propagation).to receive(:inject).with(carrier, context: context_with_span)

      adapter.inject_context(fake_span, carrier)
    end
  end
end