From 3bece1e78feadae0da593002c46aa1309823f4af Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 4 May 2023 12:46:19 +0200 Subject: [PATCH] TracingObservationHandler doesn't include manually created spans currently when you have an observation in scope and you manually create a span, then when an instrumentation happens with Observations and a child observation is created then TracingObservationHandler will assume that the parent span is not the manually created one but the parent observation which is wrong. fixes gh-244 --- ...ltTracingObservationHandlerBraveTests.java | 42 +++++++++++++++++++ ...ultTracingObservationHandlerOtelTests.java | 42 +++++++++++++++++++ .../handler/TracingObservationHandler.java | 12 +++++- 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/handler/DefaultTracingObservationHandlerBraveTests.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/handler/DefaultTracingObservationHandlerBraveTests.java index 12f974ab..9318525c 100644 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/handler/DefaultTracingObservationHandlerBraveTests.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/handler/DefaultTracingObservationHandlerBraveTests.java @@ -121,6 +121,48 @@ void should_create_parent_child_relationship_via_observations() { then(childFinishedSpan.getParentId()).isEqualTo(parentFinishedSpan.getSpanId()); } + @Test + void should_create_parent_child_relationship_via_observations_and_manual_spans() { + TestObservationRegistry registry = TestObservationRegistry.create(); + registry.observationConfig().observationHandler(handler); + + Span surprise = null; + Observation parent = Observation.start("parent", registry); + try (Observation.Scope scope = parent.openScope()) { + surprise = tracer.nextSpan().name("surprise").start(); + try (Tracer.SpanInScope scope2 = tracer.withSpan(surprise)) { + Observation child = Observation.createNotStarted("child", registry).start(); + child.scoped(() -> { + Observation grandchild = Observation.createNotStarted("grandchild", registry) + .parentObservation(child) + .start(); + grandchild.stop(); + }); + child.stop(); + } + surprise.end(); + } + parent.stop(); + + List spans = testSpanHandler.spans() + .stream() + .map(BraveFinishedSpan::fromBrave) + .collect(Collectors.toList()); + SpansAssert.then(spans).haveSameTraceId(); + FinishedSpan grandchildFinishedSpan = spans.get(0); + SpanAssert.then(grandchildFinishedSpan).hasNameEqualTo("grandchild"); + FinishedSpan childFinishedSpan = spans.get(1); + SpanAssert.then(childFinishedSpan).hasNameEqualTo("child"); + FinishedSpan surpriseSpan = spans.get(2); + SpanAssert.then(surpriseSpan).hasNameEqualTo("surprise"); + FinishedSpan parentFinishedSpan = spans.get(3); + SpanAssert.then(parentFinishedSpan).hasNameEqualTo("parent"); + + then(grandchildFinishedSpan.getParentId()).isEqualTo(childFinishedSpan.getSpanId()); + then(childFinishedSpan.getParentId()).isEqualTo(surprise.context().spanId()); + then(surprise.context().parentId()).isEqualTo(parentFinishedSpan.getSpanId()); + } + @Test void should_use_contextual_name() { Observation.Context context = new Observation.Context(); diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/handler/DefaultTracingObservationHandlerOtelTests.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/handler/DefaultTracingObservationHandlerOtelTests.java index a1613991..5efdd940 100644 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/handler/DefaultTracingObservationHandlerOtelTests.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/handler/DefaultTracingObservationHandlerOtelTests.java @@ -172,6 +172,48 @@ void should_create_parent_child_relationship_via_observations() { then(childFinishedSpan.getParentId()).isEqualTo(parentFinishedSpan.getSpanId()); } + @Test + void should_create_parent_child_relationship_via_observations_and_manual_spans() { + TestObservationRegistry registry = TestObservationRegistry.create(); + registry.observationConfig().observationHandler(handler); + + Span surprise = null; + Observation parent = Observation.start("parent", registry); + try (Observation.Scope scope = parent.openScope()) { + surprise = tracer.nextSpan().name("surprise").start(); + try (Tracer.SpanInScope scope2 = tracer.withSpan(surprise)) { + Observation child = Observation.createNotStarted("child", registry).start(); + child.scoped(() -> { + Observation grandchild = Observation.createNotStarted("grandchild", registry) + .parentObservation(child) + .start(); + grandchild.stop(); + }); + child.stop(); + } + surprise.end(); + } + parent.stop(); + + List spans = testSpanProcessor.spans() + .stream() + .map(OtelFinishedSpan::fromOtel) + .collect(Collectors.toList()); + SpansAssert.then(spans).haveSameTraceId(); + FinishedSpan grandchildFinishedSpan = spans.get(0); + SpanAssert.then(grandchildFinishedSpan).hasNameEqualTo("grandchild"); + FinishedSpan childFinishedSpan = spans.get(1); + SpanAssert.then(childFinishedSpan).hasNameEqualTo("child"); + FinishedSpan surpriseSpan = spans.get(2); + SpanAssert.then(surpriseSpan).hasNameEqualTo("surprise"); + FinishedSpan parentFinishedSpan = spans.get(3); + SpanAssert.then(parentFinishedSpan).hasNameEqualTo("parent"); + + then(grandchildFinishedSpan.getParentId()).isEqualTo(childFinishedSpan.getSpanId()); + then(childFinishedSpan.getParentId()).isEqualTo(surprise.context().spanId()); + then(surprise.context().parentId()).isEqualTo(parentFinishedSpan.getSpanId()); + } + @Test void should_use_contextual_name() { Observation.Context context = new Observation.Context(); diff --git a/micrometer-tracing/src/main/java/io/micrometer/tracing/handler/TracingObservationHandler.java b/micrometer-tracing/src/main/java/io/micrometer/tracing/handler/TracingObservationHandler.java index d3c84182..176909c5 100755 --- a/micrometer-tracing/src/main/java/io/micrometer/tracing/handler/TracingObservationHandler.java +++ b/micrometer-tracing/src/main/java/io/micrometer/tracing/handler/TracingObservationHandler.java @@ -147,12 +147,22 @@ default void onScopeClosed(T context) { default Span getParentSpan(Observation.ContextView context) { // This would mean that the user has manually created a tracing context TracingContext tracingContext = context.get(TracingContext.class); + Span currentSpan = getTracer().currentSpan(); if (tracingContext == null) { ObservationView observation = context.getParentObservation(); if (observation != null) { tracingContext = observation.getContextView().get(TracingContext.class); if (tracingContext != null) { - return tracingContext.getSpan(); + Span spanFromParentObservation = tracingContext.getSpan(); + if (spanFromParentObservation == null && currentSpan != null) { + return currentSpan; + } + else if (currentSpan != null && !currentSpan.equals(spanFromParentObservation)) { + // User manually created a span + return currentSpan; + } + // No manually created span + return spanFromParentObservation; } } }