Skip to content

Commit

Permalink
Fixed wrong OtelTraceContext sampling and parent fields resolution
Browse files Browse the repository at this point in the history
Without this change an NPE was thrown when unboxing the sampling flag. Also the parent id was resolved improperly. Lastly OTel doesn't understand a deferred sampling decision (that's why NPE was thrown, but even after fixing it sampling decision is either positive or negative).
with this change we're fixing the unboxing problem plus we're directly passing the sampling decision and parent id as respones for trace context method execution (for sampling and parent id)

fixes gh-911
  • Loading branch information
marcingrzejszczak committed Jan 8, 2025
1 parent be256d8 commit 06c616e
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2022 the original author or authors.
* Copyright 2025 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.
Expand All @@ -15,6 +15,7 @@
*/
package io.micrometer.tracing.otel.bridge;

import io.micrometer.common.lang.Nullable;
import io.micrometer.common.util.StringUtils;
import io.micrometer.tracing.TraceContext;
import io.opentelemetry.api.trace.SpanContext;
Expand All @@ -35,6 +36,7 @@ public class OtelTraceContextBuilder implements TraceContext.Builder {

private String spanId;

@Nullable
private Boolean sampled;

@Override
Expand All @@ -56,23 +58,39 @@ public TraceContext.Builder spanId(String spanId) {
}

@Override
public TraceContext.Builder sampled(Boolean sampled) {
public TraceContext.Builder sampled(@Nullable Boolean sampled) {
this.sampled = sampled;
return this;
}

@Override
public TraceContext build() {
boolean actualSampled = this.sampled != null && this.sampled;
if (StringUtils.isNotEmpty(this.parentId)) {
return new OtelTraceContext(
SpanContext.createFromRemoteParent(this.traceId, this.spanId,
this.sampled ? TraceFlags.getSampled() : TraceFlags.getDefault(), TraceState.getDefault()),
null);
actualSampled ? TraceFlags.getSampled() : TraceFlags.getDefault(), TraceState.getDefault()),
null) {
@Override
public String parentId() {
return parentId;
}

@Override
public Boolean sampled() {
return sampled;
}
};
}
return new OtelTraceContext(
SpanContext.create(this.traceId, this.spanId,
this.sampled ? TraceFlags.getSampled() : TraceFlags.getDefault(), TraceState.getDefault()),
null);
actualSampled ? TraceFlags.getSampled() : TraceFlags.getDefault(), TraceState.getDefault()),
null) {
@Override
public Boolean sampled() {
return sampled;
}
};
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* Copyright 2025 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.micrometer.tracing.TraceContext;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.Test;

class OtelTraceContextBuilderTests {

public static final String TRACE_ID = "3e425f2373d89640bde06e8285e7bf88";

public static final String PARENT_SPAN_ID = "9a5fdefae3abb440";

public static final String SPAN_ID = "ae3abb4409a5fdef";

@Test
void should_build_trace_context_for_non_null_sampled() {
TraceContext traceContext = new OtelTraceContextBuilder().traceId(TRACE_ID)
.parentId(PARENT_SPAN_ID)
.spanId(SPAN_ID)
.sampled(true)
.build();

SoftAssertions.assertSoftly(softly -> {
softly.assertThat(traceContext.spanId()).isEqualTo(SPAN_ID);
softly.assertThat(traceContext.traceId()).isEqualTo(TRACE_ID);
softly.assertThat(traceContext.parentId()).isEqualTo(PARENT_SPAN_ID);
softly.assertThat(traceContext.sampled()).isTrue();
});
}

@Test
void should_build_trace_context_for_null_sampled() {
TraceContext traceContext = new OtelTraceContextBuilder().traceId(TRACE_ID)
.parentId(PARENT_SPAN_ID)
.spanId(SPAN_ID)
.sampled(null)
.build();

SoftAssertions.assertSoftly(softly -> {
softly.assertThat(traceContext.spanId()).isEqualTo(SPAN_ID);
softly.assertThat(traceContext.traceId()).isEqualTo(TRACE_ID);
softly.assertThat(traceContext.parentId()).isEqualTo(PARENT_SPAN_ID);
softly.assertThat(traceContext.sampled()).isNull();
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public Boolean sampled() {
* @return {@code true} when sampled, {@code false} when not sampled and {@code null}
* when sampling decision should be deferred
*/
@Nullable
Boolean sampled();

/**
Expand Down Expand Up @@ -138,7 +139,7 @@ public TraceContext build() {
* @param sampled if span is sampled
* @return this
*/
TraceContext.Builder sampled(Boolean sampled);
TraceContext.Builder sampled(@Nullable Boolean sampled);

/**
* Builds the trace context.
Expand Down

0 comments on commit 06c616e

Please sign in to comment.