Skip to content

Commit

Permalink
fix: handle null parent spans in asChildOf calls
Browse files Browse the repository at this point in the history
Motivation:

The opentracing apis state that when calling the asChildOf methods with
null parameters they should behave as no-ops. The existing code was
throwing null pointers in these cases

ref: https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/Tracer.java#L150

Modifications:

- treat null values in childOf calls as no-ops

Result:

Our tracer implementation adheres to the opentracing apis
  • Loading branch information
eddie4941 committed Mar 30, 2022
1 parent b01305a commit 51487ee
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@
import io.opentracing.SpanContext;
import io.opentracing.Tracer.SpanBuilder;

import javax.annotation.Nullable;

/**
* A {@link SpanBuilder} that works with {@link InMemorySpan} instances.
*/
public interface InMemorySpanBuilder extends SpanBuilder {
@Override
InMemorySpanBuilder asChildOf(SpanContext parent);
InMemorySpanBuilder asChildOf(@Nullable SpanContext parent);

@Override
InMemorySpanBuilder asChildOf(Span parent);
InMemorySpanBuilder asChildOf(@Nullable Span parent);

/**
* {@inheritDoc}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,19 @@ abstract class AbstractInMemorySpanBuilder implements InMemorySpanBuilder {
}

@Override
public final InMemorySpanBuilder asChildOf(SpanContext parent) {
public final InMemorySpanBuilder asChildOf(@Nullable SpanContext parent) {
if (parent == null) {
return this;
}
addReference(CHILD_OF, parent);
return this;
}

@Override
public final InMemorySpanBuilder asChildOf(Span parent) {
public final InMemorySpanBuilder asChildOf(@Nullable Span parent) {
if (parent == null) {
return this;
}
addReference(CHILD_OF, parent.context());
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@
*/
package io.servicetalk.opentracing.inmemory;

import io.opentracing.SpanContext;
import io.servicetalk.opentracing.inmemory.api.InMemoryReference;
import io.servicetalk.opentracing.inmemory.api.InMemoryScopeManager;
import io.servicetalk.opentracing.inmemory.api.InMemorySpan;
import io.servicetalk.opentracing.inmemory.api.InMemorySpanBuilder;
import io.servicetalk.opentracing.inmemory.api.InMemorySpanContext;
import io.servicetalk.opentracing.inmemory.api.InMemoryTracer;

import org.junit.jupiter.api.Test;

import java.util.Optional;

import static io.opentracing.References.CHILD_OF;
import static io.opentracing.References.FOLLOWS_FROM;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand All @@ -40,6 +44,23 @@ void followsFromReferenceRespected() {
verifyParentReference("followsFromReferenceRespected", false);
}

@Test
void testChildOfNullParent() {
InMemoryScopeManager mockScopeManager = mock(InMemoryScopeManager.class);
InMemoryTracer tracer = new DefaultInMemoryTracer.Builder(mockScopeManager).build();
InMemorySpan parentSpan = tracer.buildSpan("parent")
.asChildOf((SpanContext) null)
.start();

assertEquals(Optional.empty(), findChildRef(parentSpan));

InMemorySpan childSpan = tracer.buildSpan("child")
.asChildOf(parentSpan)
.start();

assertTrue(isChildOf(childSpan, parentSpan));
}

private static void verifyParentReference(final String parentTraceIdHex, boolean childOf) {
InMemoryScopeManager mockScopeManager = mock(InMemoryScopeManager.class);
InMemorySpanContext mockParentContext = mock(InMemorySpanContext.class);
Expand All @@ -50,4 +71,20 @@ private static void verifyParentReference(final String parentTraceIdHex, boolean
InMemorySpan span = spanBuilder.start();
assertEquals(parentTraceIdHex, span.context().toTraceId());
}

private static boolean isChildOf(InMemorySpan span, InMemorySpan maybeParent) {
final InMemorySpanContext maybeParentContext = maybeParent.context();
return findChildRef(span)
.map(ref -> ref.referredTo().equals(maybeParentContext))
.orElse(false);
}

private static Optional<InMemoryReference> findChildRef(InMemorySpan span) {
for (InMemoryReference ref : span.references()) {
if (ref.type().equals(CHILD_OF)) {
return Optional.of(ref);
}
}
return Optional.empty();
}
}

0 comments on commit 51487ee

Please sign in to comment.