Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @WithSpan option to break from existing context #13112

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
Comparing source compatibility of opentelemetry-instrumentation-annotations-2.13.0-SNAPSHOT.jar against opentelemetry-instrumentation-annotations-2.12.0.jar
No changes.
**** MODIFIED ANNOTATION: PUBLIC ABSTRACT io.opentelemetry.instrumentation.annotations.WithSpan (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++* NEW METHOD: PUBLIC(+) ABSTRACT(+) boolean withParent()
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,7 @@

/** Specify the {@link SpanKind} of span to be created. Defaults to {@link SpanKind#INTERNAL}. */
SpanKind kind() default SpanKind.INTERNAL;

/** Specify whether to create a new span with parent. Defaults to {@code true}. */
boolean withParent() default true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@

import application.io.opentelemetry.instrumentation.annotations.WithSpan;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.annotation.support.MethodSpanAttributesExtractor;
import io.opentelemetry.instrumentation.api.annotation.support.SpanAttributesExtractor;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.semconv.util.SpanNames;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Method;
import java.util.logging.Logger;

Expand All @@ -29,6 +34,21 @@ public final class AnnotationSingletons {
createInstrumenterWithAttributes();
private static final SpanAttributesExtractor ATTRIBUTES = createAttributesExtractor();

// The reason for using reflection here is that it needs to be compatible with the old version of
// @WithSpan annotation that does not include the withParent option to avoid failing the muzzle
// check.
private static MethodHandle withParentMethodHandle = null;

static {
try {
withParentMethodHandle =
MethodHandles.publicLookup()
.findVirtual(WithSpan.class, "withParent", MethodType.methodType(boolean.class));
} catch (NoSuchMethodException | IllegalAccessException ignore) {
// ignore
}
}

public static Instrumenter<Method, Object> instrumenter() {
return INSTRUMENTER;
}
Expand All @@ -47,6 +67,7 @@ private static Instrumenter<Method, Object> createInstrumenter() {
INSTRUMENTATION_NAME,
AnnotationSingletons::spanNameFromMethod)
.addAttributesExtractor(CodeAttributesExtractor.create(MethodCodeAttributesGetter.INSTANCE))
.addContextCustomizer(AnnotationSingletons::parentContextFromMethod)
.buildInstrumenter(AnnotationSingletons::spanKindFromMethod);
}

Expand All @@ -62,6 +83,7 @@ private static Instrumenter<MethodRequest, Object> createInstrumenterWithAttribu
MethodRequest::method,
WithSpanParameterAttributeNamesExtractor.INSTANCE,
MethodRequest::args))
.addContextCustomizer(AnnotationSingletons::parentContextFromMethodRequest)
.buildInstrumenter(AnnotationSingletons::spanKindFromMethodRequest);
}

Expand Down Expand Up @@ -104,5 +126,28 @@ private static String spanNameFromMethod(Method method) {
return spanName;
}

private static Context parentContextFromMethodRequest(
Context context, MethodRequest request, Attributes attributes) {
return parentContextFromMethod(context, request.method(), attributes);
}

private static Context parentContextFromMethod(
Context context, Method method, Attributes attributes) {
if (withParentMethodHandle == null) {
return context;
}

WithSpan annotation = method.getDeclaredAnnotation(WithSpan.class);

boolean withParent = true;
try {
withParent = (boolean) withParentMethodHandle.invoke(annotation);
} catch (Throwable ignore) {
// ignore
}

return withParent ? context : Context.root();
}

private AnnotationSingletons() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,14 @@ public CompletionStage<String> completionStage(CompletableFuture<String> future)
public CompletableFuture<String> completableFuture(CompletableFuture<String> future) {
return future;
}

@WithSpan(withParent = false)
public String withoutParent() {
return "hello!";
}

@WithSpan(kind = SpanKind.CONSUMER)
public String consumer() {
return withoutParent();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,31 @@ void multipleSpans() {
equalTo(CODE_FUNCTION, "otel"))));
}

@Test
void multipleSpansWithoutParent() {
new TracedWithSpan().consumer();

testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("TracedWithSpan.consumer")
.hasKind(SpanKind.CONSUMER)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(CODE_NAMESPACE, TracedWithSpan.class.getName()),
equalTo(CODE_FUNCTION, "consumer"))),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("TracedWithSpan.withoutParent")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(CODE_NAMESPACE, TracedWithSpan.class.getName()),
equalTo(CODE_FUNCTION, "withoutParent"))));
}

@Test
void excludedMethod() throws Exception {
new TracedWithSpan().ignored();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.annotations.WithSpan;
import io.opentelemetry.instrumentation.api.semconv.util.SpanNames;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Method;
import javax.annotation.Nullable;
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.reflect.MethodSignature;

Expand All @@ -18,8 +22,10 @@ final class JoinPointRequest {
private final Method method;
private final String spanName;
private final SpanKind spanKind;
private final boolean withParent;

private JoinPointRequest(JoinPoint joinPoint, Method method, String spanName, SpanKind spanKind) {
private JoinPointRequest(
JoinPoint joinPoint, Method method, String spanName, SpanKind spanKind, boolean withParent) {
if (spanName.isEmpty()) {
spanName = SpanNames.fromMethod(method);
}
Expand All @@ -28,6 +34,7 @@ private JoinPointRequest(JoinPoint joinPoint, Method method, String spanName, Sp
this.method = method;
this.spanName = spanName;
this.spanKind = spanKind;
this.withParent = withParent;
}

String spanName() {
Expand All @@ -46,13 +53,32 @@ Object[] args() {
return joinPoint.getArgs();
}

boolean withParent() {
return withParent;
}

interface Factory {

JoinPointRequest create(JoinPoint joinPoint);
}

static final class InstrumentationAnnotationFactory implements Factory {

// The reason for using reflection here is that it needs to be compatible with the old version
// of @WithSpan annotation that does not include the withParent option to avoid failing the
// muzzle check.
private static MethodHandle withParentMethodHandle = null;

static {
try {
withParentMethodHandle =
MethodHandles.publicLookup()
.findVirtual(WithSpan.class, "withParent", MethodType.methodType(boolean.class));
} catch (NoSuchMethodException | IllegalAccessException ignore) {
// ignore
}
}

@Override
public JoinPointRequest create(JoinPoint joinPoint) {
MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature();
Expand All @@ -66,7 +92,20 @@ public JoinPointRequest create(JoinPoint joinPoint) {
String spanName = annotation != null ? annotation.value() : "";
SpanKind spanKind = annotation != null ? annotation.kind() : SpanKind.INTERNAL;

return new JoinPointRequest(joinPoint, method, spanName, spanKind);
return new JoinPointRequest(
joinPoint, method, spanName, spanKind, withParentValueFrom(annotation));
}

private static boolean withParentValueFrom(@Nullable WithSpan annotation) {
if (annotation == null || withParentMethodHandle == null) {
return true;
}

try {
return (boolean) withParentMethodHandle.invoke(annotation);
} catch (Throwable ignore) {
return true;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.annotations;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
Expand Down Expand Up @@ -53,10 +54,16 @@ abstract class WithSpanAspect {
JoinPointRequest::method,
parameterAttributeNamesExtractor,
JoinPointRequest::args))
.addContextCustomizer(WithSpanAspect::parentContext)
.buildInstrumenter(JoinPointRequest::spanKind);
this.requestFactory = requestFactory;
}

private static Context parentContext(
Context parentContext, JoinPointRequest request, Attributes unused) {
return request.withParent() ? parentContext : Context.root();
}

public Object traceMethod(ProceedingJoinPoint pjp) throws Throwable {
JoinPointRequest request = requestFactory.create(pjp);
Context parentContext = Context.current();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,27 @@ void withSpanAttributes() {
equalTo(stringKey("explicitName"), "baz"))));
}

@Test
@DisplayName(
"when method is annotated with @WithSpan(withParent=false) should build span without parent")
void withSpanWithoutParent() {
// when
testing.runWithSpan("parent", withSpanTester::testWithoutParentSpan);

// then
testing.waitAndAssertTraces(
trace -> trace.hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL)),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName(unproxiedTesterSimpleClassName + ".testWithoutParentSpan")
.hasKind(INTERNAL)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(CODE_NAMESPACE, unproxiedTesterClassName),
equalTo(CODE_FUNCTION, "testWithoutParentSpan"))));
}

static class InstrumentationWithSpanTester {
@WithSpan
public String testWithSpan() {
Expand Down Expand Up @@ -221,6 +242,11 @@ public String withSpanAttributes(

return "hello!";
}

@WithSpan(withParent = false)
public String testWithoutParentSpan() {
return "Span without parent span was created";
}
}

@Nested
Expand Down
Loading