From 7980543be0d5c145ef1fd4a96dcc0e0c103c844f Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 21 May 2024 13:40:37 +0200 Subject: [PATCH 1/2] Assumes that INVALID parent span id is actually null in TraceContext - without this change we're returning an INVALID parent span even though we define in the javadocs of TraceContext that we would return null - with this change we're verifying whether the INVALID parent is set in which case we're returning null fixes gh-687 --- .../tracing/otel/bridge/OtelTraceContext.java | 12 ++-- .../otel/bridge/OtelTraceContextTests.java | 68 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelTraceContext.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelTraceContext.java index 816ae520..6e018440 100644 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelTraceContext.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/main/java/io/micrometer/tracing/otel/bridge/OtelTraceContext.java @@ -15,9 +15,6 @@ */ package io.micrometer.tracing.otel.bridge; -import java.util.Objects; -import java.util.concurrent.atomic.AtomicReference; - import io.micrometer.common.lang.Nullable; import io.micrometer.tracing.TraceContext; import io.opentelemetry.api.trace.Span; @@ -25,6 +22,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.ReadableSpan; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; + /** * OpenTelemetry implementation of a {@link TraceContext}. * @@ -116,7 +116,11 @@ public String parentId() { : this.span; if (spanContextSpanOrSpan instanceof ReadableSpan) { ReadableSpan readableSpan = (ReadableSpan) spanContextSpanOrSpan; - return readableSpan.toSpanData().getParentSpanId(); + String parentSpanId = readableSpan.toSpanData().getParentSpanId(); + if (Objects.equals(Span.getInvalid().getSpanContext().getSpanId(), parentSpanId)) { + return null; + } + return parentSpanId; } return null; } diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java new file mode 100644 index 00000000..82fb13ff --- /dev/null +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java @@ -0,0 +1,68 @@ +/** + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.micrometer.tracing.otel.bridge; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.OpenTelemetrySdkBuilder; +import io.opentelemetry.sdk.trace.SdkTracerProvider; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.BDDAssertions.then; + +class OtelTraceContextTests { + + SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() + .setSampler(io.opentelemetry.sdk.trace.samplers.Sampler.alwaysOn()) + .build(); + + OpenTelemetrySdkBuilder openTelemetrySdkBuilder = OpenTelemetrySdk.builder().setTracerProvider(sdkTracerProvider); + + @Test + void should_return_null_when_parent_invalid() { + + try (OpenTelemetrySdk openTelemetrySdk = openTelemetrySdkBuilder.build()) { + Tracer otelTracer = tracer(openTelemetrySdk); + Span span = otelTracer.spanBuilder("foo").startSpan(); + + OtelTraceContext otelTraceContext = new OtelTraceContext(span); + + then(otelTraceContext.parentId()).isNull(); + } + + } + + @Test + void should_return_parentid_when_parent_valid() { + try (OpenTelemetrySdk openTelemetrySdk = openTelemetrySdkBuilder.build()) { + Tracer otelTracer = tracer(openTelemetrySdk); + Span parentSpan = otelTracer.spanBuilder("parent").startSpan(); + Span span = otelTracer.spanBuilder("foo") + .setParent(parentSpan.storeInContext(Context.current())) + .startSpan(); + + OtelTraceContext otelTraceContext = new OtelTraceContext(span); + + then(otelTraceContext.parentId()).isEqualTo(parentSpan.getSpanContext().getSpanId()); + } + + } + + private static Tracer tracer(OpenTelemetrySdk openTelemetrySdk) { + return openTelemetrySdk.getTracer("io.micrometer.micrometer-tracing"); + } + +} From e01c8d66b92149d07fb0f8eca3b20dcd3ff4884f Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 24 May 2024 09:27:44 +0200 Subject: [PATCH 2/2] OTel doesn't store parent id for non sampled spans --- .../otel/bridge/OtelTraceContextTests.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java index 82fb13ff..79084dc6 100644 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-otel/src/test/java/io/micrometer/tracing/otel/bridge/OtelTraceContextTests.java @@ -45,6 +45,29 @@ void should_return_null_when_parent_invalid() { } + @Test + void should_return_null_when_spans_not_sampled() { // works differently than Brave + SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() + .setSampler(io.opentelemetry.sdk.trace.samplers.Sampler.alwaysOff()) + .build(); + + OpenTelemetrySdkBuilder openTelemetrySdkBuilder = OpenTelemetrySdk.builder() + .setTracerProvider(sdkTracerProvider); + + try (OpenTelemetrySdk openTelemetrySdk = openTelemetrySdkBuilder.build()) { + Tracer otelTracer = tracer(openTelemetrySdk); + Span parentSpan = otelTracer.spanBuilder("parent").startSpan(); + Span span = otelTracer.spanBuilder("foo") + .setParent(parentSpan.storeInContext(Context.current())) + .startSpan(); + + OtelTraceContext otelTraceContext = new OtelTraceContext(span); + + then(otelTraceContext.parentId()).isNull(); + } + + } + @Test void should_return_parentid_when_parent_valid() { try (OpenTelemetrySdk openTelemetrySdk = openTelemetrySdkBuilder.build()) {