Clean up and document BaseTracer
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java
@@ -23,16 +23,22 @@ import io.opentelemetry.instrumentation.api.context.ContextPropagationDebug;
import java.lang.reflect.Method;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Base class for all instrumentation specific tracer implementations.
*
* <p>Tracers should not use {@link Span} directly in their public APIs: ideally all lifecycle
- * methods (ex. start/end methods) should return/accept {@link Context}.
+ * methods (ex. start/end methods) should return/accept {@link Context}. By convention, {@link
+ * Context} should be passed to all methods as the first parameter.
*
* <p>The {@link BaseTracer} offers several {@code startSpan()} utility methods for creating bare
* spans without any attributes. If you want to provide some additional attributes on span start
- * please consider writing your own specific {@code startSpan()} method in the your tracer.
+ * please consider writing your own specific {@code startSpan()} method in your tracer.
+ *
+ * <p>A {@link Context} returned by any {@code startSpan()} method will <b>always</b> contain a new
+ * span. If there is a need to suppress span creation {@link #shouldStartSpan(Context, SpanKind)}
+ * should be called before {@code startSpan()}.
*
* <p>When constructing {@link Span}s tracers should set all attributes available during
* construction on a {@link SpanBuilder} instead of a {@link Span}. This way {@code SpanProcessor}s
@@ -67,40 +73,40 @@ public abstract class BaseTracer {
this.propagators = openTelemetry.getPropagators();
}
- public Context startSpan(Class<?> clazz) {
- return startSpan(spanNameForClass(clazz));
- }
-
- public Context startSpan(Method method) {
- return startSpan(spanNameForMethod(method));
- }
-
- public Context startSpan(String spanName) {
- return startSpan(spanName, SpanKind.INTERNAL);
- }
-
- public Context startSpan(String spanName, SpanKind kind) {
- return startSpan(Context.current(), spanName, kind);
- }
-
- public Context startSpan(Context parentContext, String spanName, SpanKind kind) {
- Span span = spanBuilder(spanName, kind).setParent(parentContext).startSpan();
- return parentContext.with(span);
- }
-
- protected SpanBuilder spanBuilder(String spanName, SpanKind kind) {
- return tracer.spanBuilder(spanName).setSpanKind(kind);
- }
-
- protected final Context withClientSpan(Context parentContext, Span span) {
- return ClientSpan.with(parentContext.with(span), span);
- }
+ /**
+ * The name of the instrumentation library, not the name of the instrument*ed* library. The value
+ * returned by this method should uniquely identify the instrumentation library so that during
+ * troubleshooting it's possible to pinpoint what tracer produced problematic telemetry.
+ *
+ * <p>In this project we use a convention to encode the version of the instrument*ed* library into
+ * the instrumentation name, for example {@code io.opentelemetry.javaagent.apache-httpclient-4.0}.
+ * This way, if there are different instrumentations for different library versions it's easy to
+ * find out which instrumentations produced the telemetry data.
+ *
+ * @see io.opentelemetry.api.trace.TracerProvider#get(String, String)
+ */
+ protected abstract String getInstrumentationName();
- protected final Context withServerSpan(Context parentContext, Span span) {
- return ServerSpan.with(parentContext.with(span), span);
+ /**
+ * The version of the instrumentation library - defaults to the value of JAR manifest attribute
+ * {@code Implementation-Version}.
+ */
+ protected String getVersion() {
+ return InstrumentationVersion.VERSION;
}
- protected final boolean shouldStartSpan(SpanKind proposedKind, Context context) {
+ /**
+ * Returns true if a new span of the {@code proposedKind} should be suppressed.
+ *
+ * <p>If the passed {@code context} contains a {@link SpanKind#SERVER} span the instrumentation
+ * must not create another {@code SERVER} span. The same is true for a {@link SpanKind#CLIENT}
+ * span: if one {@code CLIENT} span is already present in the passed {@code context} then another
+ * one must not be started.
+ *
+ * @see #withClientSpan(Context, Span)
+ * @see #withServerSpan(Context, Span)
+ */
+ public final boolean shouldStartSpan(Context context, SpanKind proposedKind) {
boolean suppressed = false;
switch (proposedKind) {
case CLIENT:
@@ -126,32 +132,76 @@ public abstract class BaseTracer {
return ServerSpan.fromContextOrNull(context) != null;
}
- protected abstract String getInstrumentationName();
+ /**
+ * Returns a {@link Context} inheriting from {@code Context.current()} that contains a new span
+ * with name {@code spanName} and kind {@link SpanKind#INTERNAL}.
+ */
+ public Context startSpan(String spanName) {
+ return startSpan(spanName, SpanKind.INTERNAL);
+ }
- protected String getVersion() {
- return InstrumentationVersion.VERSION;
+ /**
+ * Returns a {@link Context} inheriting from {@code Context.current()} that contains a new span
+ * with name {@code spanName} and kind {@code kind}.
+ */
+ public Context startSpan(String spanName, SpanKind kind) {
+ return startSpan(Context.current(), spanName, kind);
+ }
+
+ /**
+ * Returns a {@link Context} inheriting from {@code parentContext} that contains a new span with
+ * name {@code spanName} and kind {@code kind}.
+ */
+ public Context startSpan(Context parentContext, String spanName, SpanKind kind) {
+ Span span = spanBuilder(spanName, kind).setParent(parentContext).startSpan();
+ return parentContext.with(span);
+ }
+
+ protected SpanBuilder spanBuilder(String spanName, SpanKind kind) {
+ return tracer.spanBuilder(spanName).setSpanKind(kind);
+ }
+
+ /**
+ * Returns a {@link Context} containing the passed {@code span} marked as the current {@link
+ * SpanKind#CLIENT} span.
+ *
+ * @see #shouldStartSpan(Context, SpanKind)
+ */
+ protected final Context withClientSpan(Context parentContext, Span span) {
+ return ClientSpan.with(parentContext.with(span), span);
+ }
+
+ /**
+ * Returns a {@link Context} containing the passed {@code span} marked as the current {@link
+ * SpanKind#SERVER} span.
+ *
+ * @see #shouldStartSpan(Context, SpanKind)
+ */
+ protected final Context withServerSpan(Context parentContext, Span span) {
+ return ServerSpan.with(parentContext.with(span), span);
}
/**
* This method is used to generate an acceptable span (operation) name based on a given method
* reference. Anonymous classes are named based on their parent.
*/
- public String spanNameForMethod(Method method) {
+ public static String spanNameForMethod(Method method) {
return spanNameForMethod(method.getDeclaringClass(), method.getName());
}
/**
* This method is used to generate an acceptable span (operation) name based on a given method
* reference. Anonymous classes are named based on their parent.
- *
- * @param method the method to get the name from, nullable
- * @return the span name from the class and method
*/
- protected String spanNameForMethod(Class<?> clazz, Method method) {
- return spanNameForMethod(clazz, null == method ? null : method.getName());
+ public static String spanNameForMethod(Class<?> clazz, @Nullable Method method) {
+ return spanNameForMethod(clazz, method == null ? "<unknown>" : method.getName());
}
- protected String spanNameForMethod(Class<?> cl, String methodName) {
+ /**
+ * This method is used to generate an acceptable span (operation) name based on a given method
+ * reference. Anonymous classes are named based on their parent.
+ */
+ public static String spanNameForMethod(Class<?> cl, String methodName) {
return spanNameForClass(cl) + "." + methodName;
}
@@ -159,7 +209,7 @@ public abstract class BaseTracer {
* This method is used to generate an acceptable span (operation) name based on a given class
* reference. Anonymous classes are named based on their parent.
*/
- public String spanNameForClass(Class<?> clazz) {
+ public static String spanNameForClass(Class<?> clazz) {
if (!clazz.isAnonymousClass()) {
return clazz.getSimpleName();
}
@@ -173,15 +223,18 @@ public abstract class BaseTracer {
return className;
}
+ /** Ends the execution of a span stored in the passed {@code context}. */
public void end(Context context) {
end(context, -1);
}
+ /**
+ * Ends the execution of a span stored in the passed {@code context}.
+ *
+ * @param endTimeNanos Explicit nanoseconds timestamp from the epoch.
+ */
public void end(Context context, long endTimeNanos) {
- end(Span.fromContext(context), endTimeNanos);
- }
-
- private void end(Span span, long endTimeNanos) {
+ Span span = Span.fromContext(context);
if (endTimeNanos > 0) {
span.end(endTimeNanos, TimeUnit.NANOSECONDS);
} else {
@@ -189,18 +242,25 @@ public abstract class BaseTracer {
}
}
+ /**
+ * Records the {@code throwable} in the span stored in the passed {@code context} and marks the
+ * end of the span's execution.
+ */
public void endExceptionally(Context context, Throwable throwable) {
endExceptionally(context, throwable, -1);
}
+ /**
+ * Records the {@code throwable} in the span stored in the passed {@code context} and marks the
+ * end of the span's execution.
+ *
+ * @param endTimeNanos Explicit nanoseconds timestamp from the epoch.
+ */
public void endExceptionally(Context context, Throwable throwable, long endTimeNanos) {
- endExceptionally(Span.fromContext(context), throwable, endTimeNanos);
- }
-
- private void endExceptionally(Span span, Throwable throwable, long endTimeNanos) {
+ Span span = Span.fromContext(context);
span.setStatus(StatusCode.ERROR);
onError(span, unwrapThrowable(throwable));
- end(span, endTimeNanos);
+ end(context, endTimeNanos);
}
protected void onError(Span span, Throwable throwable) {
@@ -211,10 +271,17 @@ public abstract class BaseTracer {
return throwable instanceof ExecutionException ? throwable.getCause() : throwable;
}
+ // TODO: call onError instead and make this private
public void addThrowable(Span span, Throwable throwable) {
span.recordException(throwable);
}
+ /**
+ * Extracts a {@link Context} from {@code carrier} using the propagator embedded in this tracer.
+ * This method can be used to propagate {@link Context} passed from upstream services.
+ *
+ * @see TextMapPropagator#extract(Context, Object, TextMapGetter)
+ */
public <C> Context extract(C carrier, TextMapGetter<C> getter) {
ContextPropagationDebug.debugContextLeakIfEnabled();

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java
@@ -33,7 +33,7 @@ public abstract class DatabaseClientTracer<CONNECTION, STATEMENT, SANITIZEDSTATE
}
public boolean shouldStartSpan(Context parentContext) {
- return shouldStartSpan(CLIENT, parentContext);
+ return shouldStartSpan(parentContext, CLIENT);
}
public Context startSpan(Context parentContext, CONNECTION connection, STATEMENT statement) {

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java
@@ -73,7 +73,7 @@ public abstract class HttpClientTracer<REQUEST, CARRIER, RESPONSE> extends BaseT
protected abstract TextMapSetter<CARRIER> getSetter();
public boolean shouldStartSpan(Context parentContext) {
- return shouldStartSpan(CLIENT, parentContext);
+ return shouldStartSpan(parentContext, CLIENT);
}
public Context startSpan(Context parentContext, REQUEST request, CARRIER carrier) {

MODIFY
instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseTracerTest.groovy
instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseTracerTest.groovy
@@ -38,7 +38,7 @@ class BaseTracerTest extends Specification {
def "test shouldStartSpan"() {
when:
- boolean result = tracer.shouldStartSpan(kind, context)
+ boolean result = tracer.shouldStartSpan(context, kind)
then:
result == expected

MODIFY
instrumentation/external-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/extannotations/TraceAnnotationTracer.java
instrumentation/external-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/extannotations/TraceAnnotationTracer.java
package io.opentelemetry.javaagent.instrumentation.extannotations;
+import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
+import java.lang.reflect.Method;
public class TraceAnnotationTracer extends BaseTracer {
private static final TraceAnnotationTracer TRACER = new TraceAnnotationTracer();
@@ -18,4 +20,8 @@ public class TraceAnnotationTracer extends BaseTracer {
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.external-annotations";
}
+
+ public Context startSpan(Method method) {
+ return startSpan(spanNameForMethod(method));
+ }
}

MODIFY
instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodTracer.java
instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodTracer.java
package io.opentelemetry.javaagent.instrumentation.methods;
+import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
+import java.lang.reflect.Method;
public class MethodTracer extends BaseTracer {
private static final MethodTracer TRACER = new MethodTracer();
@@ -18,4 +20,8 @@ public class MethodTracer extends BaseTracer {
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.external-annotations";
}
+
+ public Context startSpan(Method method) {
+ return startSpan(spanNameForMethod(method));
+ }
}

MODIFY
instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java
instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java
@@ -30,6 +30,8 @@ public class WithSpanAdvice {
SpanKind kind = tracer().extractSpanKind(applicationAnnotation);
Context current = Context.current();
+
+ // don't create a nested span if you're not supposed to.
if (tracer().shouldStartSpan(current, kind)) {
context = tracer().startSpan(current, applicationAnnotation, method, kind);
scope = context.makeCurrent();

MODIFY
instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java
instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java
@@ -23,13 +23,6 @@ public class WithSpanTracer extends BaseTracer {
private static final Logger log = LoggerFactory.getLogger(WithSpanTracer.class);
- // we can't conditionally start a span in startSpan() below, because the caller won't know
- // whether to call end() or not on the Span in the returned Context
- public boolean shouldStartSpan(Context context, SpanKind kind) {
- // don't create a nested span if you're not supposed to.
- return shouldStartSpan(kind, context);
- }
-
public Context startSpan(
Context context, WithSpan applicationAnnotation, Method method, SpanKind kind) {
Span span =

MODIFY
instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientTracer.java
instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientTracer.java
@@ -20,7 +20,6 @@ public class RmiClientTracer extends RpcClientTracer {
return TRACER;
}
- @Override
public Context startSpan(Method method) {
String serviceName = method.getDeclaringClass().getName();
String methodName = method.getName();

MODIFY
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/dispatcher/RequestDispatcherTracer.java
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/dispatcher/RequestDispatcherTracer.java
package io.opentelemetry.javaagent.instrumentation.servlet.dispatcher;
+import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
+import java.lang.reflect.Method;
public class RequestDispatcherTracer extends BaseTracer {
private static final RequestDispatcherTracer TRACER = new RequestDispatcherTracer();
@@ -18,4 +20,8 @@ public class RequestDispatcherTracer extends BaseTracer {
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.servlet-common";
}
+
+ public Context startSpan(Method method) {
+ return startSpan(spanNameForMethod(method));
+ }
}

MODIFY
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletResponseTracer.java
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletResponseTracer.java
package io.opentelemetry.javaagent.instrumentation.servlet.http;
+import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
+import java.lang.reflect.Method;
public class HttpServletResponseTracer extends BaseTracer {
private static final HttpServletResponseTracer TRACER = new HttpServletResponseTracer();
@@ -18,4 +20,8 @@ public class HttpServletResponseTracer extends BaseTracer {
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.servlet-common";
}
+
+ public Context startSpan(Method method) {
+ return startSpan(spanNameForMethod(method));
+ }
}

MODIFY
instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataTracer.java
instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataTracer.java
package io.opentelemetry.javaagent.instrumentation.spring.data;
+import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
+import java.lang.reflect.Method;
public final class SpringDataTracer extends BaseTracer {
private static final SpringDataTracer TRACER = new SpringDataTracer();
@@ -20,4 +22,8 @@ public final class SpringDataTracer extends BaseTracer {
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-data-1.8";
}
+
+ public Context startSpan(Method method) {
+ return startSpan(spanNameForMethod(method));
+ }
}

MODIFY
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java
@@ -34,7 +34,7 @@ public class TwilioTracer extends BaseTracer {
}
public boolean shouldStartSpan(Context parentContext) {
- return shouldStartSpan(CLIENT, parentContext);
+ return shouldStartSpan(parentContext, CLIENT);
}
public Context startSpan(Context parentContext, Object serviceExecutor, String methodName) {
@@ -49,7 +49,7 @@ public class TwilioTracer extends BaseTracer {
/** Decorate trace based on service execution metadata. */
private String spanNameOnServiceExecution(Object serviceExecutor, String methodName) {
- return spanNameForClass(serviceExecutor.getClass()) + "." + methodName;
+ return spanNameForMethod(serviceExecutor.getClass(), methodName);
}
/** Annotate the span with the results of the operation. */

Clean up and document BaseTracer (#2482)