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

Refactor span names class #3281

Merged
merged 9 commits into from
Jun 14, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -6,7 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter.code;

import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import io.opentelemetry.instrumentation.api.tracer.ClassNames;

/**
* A helper {@link SpanNameExtractor} implementation for instrumentations that target specific Java
Expand All @@ -32,8 +32,7 @@ private CodeSpanNameExtractor(CodeAttributesExtractor<REQUEST, ?> attributesExtr
@Override
public String extract(REQUEST request) {
Class<?> cls = attributesExtractor.codeClass(request);
// TODO: avoid using SpanNames, encapsulate the logic here
String className = cls != null ? SpanNames.spanNameForClass(cls) : "<unknown>";
String className = cls != null ? ClassNames.simpleName(cls) : "<unknown>";
String methodName = defaultString(attributesExtractor.methodName(request));
return className + "." + methodName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@
import io.opentelemetry.instrumentation.api.internal.ContextPropagationDebug;
import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.concurrent.CompletionException;
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.
Expand Down Expand Up @@ -178,50 +176,6 @@ 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.
*
* @deprecated Use {@link SpanNames#spanNameForMethod(Method)}.
*/
@Deprecated
public static String spanNameForMethod(Method method) {
return SpanNames.spanNameForMethod(method);
}

/**
* 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.
*
* @deprecated Use {@link SpanNames#spanNameForMethod(Class, Method)}.
*/
@Deprecated
public static String spanNameForMethod(Class<?> clazz, @Nullable Method method) {
return SpanNames.spanNameForMethod(clazz, method);
}

/**
* 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.
*
* @deprecated Use {@link SpanNames#spanNameForMethod(Class, String)}.
*/
@Deprecated
public static String spanNameForMethod(Class<?> cl, String methodName) {
return SpanNames.spanNameForMethod(cl, methodName);
}

/**
* 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.
*
* @deprecated Use {@link SpanNames#spanNameForClass(Class)}.
*/
@Deprecated
public static String spanNameForClass(Class<?> clazz) {
return SpanNames.spanNameForClass(clazz);
}

/** Ends the execution of a span stored in the passed {@code context}. */
public void end(Context context) {
end(context, -1);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.tracer;

public final class ClassNames {

private static final ClassValue<String> simpleNames =
new ClassValue<String>() {
@Override
protected String computeValue(Class<?> type) {
if (!type.isAnonymousClass()) {
return type.getSimpleName();
}
String className = type.getName();
if (type.getPackage() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Packages are optional. Perhaps it would be better to use the same strategy as jdk uses

className = className.substring(className.lastIndexOf('.') + 1);

String pkgName = type.getPackage().getName();
if (!pkgName.isEmpty()) {
className = className.substring(pkgName.length() + 1);
}
}
return className;
}
};

/**
* This method is used to generate a simple name based on a given class reference, e.g. for use in
* span names and span attributes. Anonymous classes are named based on their parent.
*/
public static String simpleName(Class<?> type) {
return simpleNames.get(type);
}

private ClassNames() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected HttpServerTracer(OpenTelemetry openTelemetry) {
}

public Context startSpan(REQUEST request, CONNECTION connection, STORAGE storage, Method origin) {
String spanName = spanNameForMethod(origin);
String spanName = SpanNames.fromMethod(origin);
return startSpan(request, connection, storage, spanName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,45 +10,27 @@

public final class SpanNames {
/**
* 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.
* This method is used to generate a span name based on a method. Anonymous classes are named
* based on their parent.
*/
public static String spanNameForMethod(Method method) {
return spanNameForMethod(method.getDeclaringClass(), method.getName());
public static String fromMethod(Method method) {
return fromMethod(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.
* This method is used to generate a span name based on a method. Anonymous classes are named
* based on their parent.
*/
public static String spanNameForMethod(Class<?> clazz, @Nullable Method method) {
return spanNameForMethod(clazz, method == null ? "<unknown>" : method.getName());
public static String fromMethod(Class<?> clazz, @Nullable Method method) {
return fromMethod(clazz, method == null ? "<unknown>" : 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.
* This method is used to generate a span name based on a method. Anonymous classes are named
* based on their parent.
*/
public static String spanNameForMethod(Class<?> cl, String methodName) {
return spanNameForClass(cl) + "." + methodName;
}

/**
* 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 static String spanNameForClass(Class<?> clazz) {
if (!clazz.isAnonymousClass()) {
return clazz.getSimpleName();
}
String className = clazz.getName();
if (clazz.getPackage() != null) {
String pkgName = clazz.getPackage().getName();
if (!pkgName.isEmpty()) {
className = className.substring(pkgName.length() + 1);
}
}
return className;
public static String fromMethod(Class<?> cl, String methodName) {
return ClassNames.simpleName(cl) + "." + methodName;
}

private SpanNames() {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.tracer

import spock.lang.Specification

class ClassNamesTest extends Specification {

def "test simpleName"() {
when:
String result = ClassNames.simpleName(clazz)

then:
result == expected

where:
clazz | expected
ClassNamesTest | "ClassNamesTest"
ClassNames | "ClassNames"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,42 @@ import spock.lang.Specification

class SpanNamesTest extends Specification {

def "test spanNameForClass"() {
def "test fromMethod"() {
when:
String result = SpanNames.spanNameForClass(clazz)
String result = SpanNames.fromMethod(method)

then:
result == expected

where:
clazz | expected
SpanNamesTest | "SpanNamesTest"
SpanNames | "SpanNames"
method | expected
ReflectionUtil.getMethodByName(SpanNames, "fromMethod") | "SpanNames.fromMethod"
ReflectionUtil.getMethodByName(String, "length") | "String.length"
}

def "test spanNameForMethod"() {
def "test fromMethod with class and method ref"() {
when:
String result = SpanNames.spanNameForMethod(method)
String result = SpanNames.fromMethod(clazz, method)

then:
result == expected

where:
method | expected
ReflectionUtil.getMethodByName(SpanNames, "spanNameForClass") | "SpanNames.spanNameForClass"
ReflectionUtil.getMethodByName(String, "length") | "String.length"
clazz = SpanNames
method = ReflectionUtil.getMethodByName(SpanNames, "fromMethod")
expected = "SpanNames.fromMethod"
}

def "test spanNameForMethod with class"() {
def "test fromMethod with class and method name"() {
when:
String result = SpanNames.spanNameForMethod(clazz, method)
String result = SpanNames.fromMethod(clazz, method)

then:
result == expected

where:
clazz | method | expected
SpanNames | ReflectionUtil.getMethodByName(SpanNames, "spanNameForClass") | "SpanNames.spanNameForClass"
SpanNames | "test" | "SpanNames.test"
clazz = SpanNames
method = "test"
expected = "SpanNames.test"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import java.lang.reflect.Method;

public class ExternalAnnotationTracer extends BaseTracer {
Expand All @@ -22,6 +23,6 @@ protected String getInstrumentationName() {
}

public Context startSpan(Method method) {
return startSpan(spanNameForMethod(method));
return startSpan(SpanNames.fromMethod(method));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.ClassNames;

public class FinatraTracer extends BaseTracer {
private static final FinatraTracer TRACER = new FinatraTracer();
Expand All @@ -22,6 +23,6 @@ protected String getInstrumentationName() {
}

public Context startSpan(Context parentContext, Class<?> clazz) {
return super.startSpan(parentContext, spanNameForClass(clazz), SpanKind.INTERNAL);
return super.startSpan(parentContext, ClassNames.simpleName(clazz), SpanKind.INTERNAL);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import org.grails.web.mapping.mvc.GrailsControllerUrlMappingInfo;

public class GrailsTracer extends BaseTracer {
Expand All @@ -22,7 +23,7 @@ public static GrailsTracer tracer() {
}

public Context startSpan(Object controller, String action) {
return startSpan(spanNameForClass(controller.getClass()) + "." + action);
return startSpan(SpanNames.fromMethod(controller.getClass(), action));
}

public void updateServerSpanName(Context context, GrailsControllerUrlMappingInfo info) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import io.opentelemetry.javaagent.instrumentation.api.ClassHierarchyIterable;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.lang.annotation.Annotation;
Expand Down Expand Up @@ -47,7 +48,7 @@ public Context startSpan(Class<?> target, Method method) {
if (serverSpan == null) {
spanName = pathBasedSpanName;
} else {
spanName = spanNameForMethod(target, method);
spanName = SpanNames.fromMethod(target, method);
updateServerSpanName(parentContext, serverSpan, pathBasedSpanName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import io.opentelemetry.javaagent.instrumentation.api.ClassHierarchyIterable;
import io.opentelemetry.javaagent.instrumentation.api.jaxrs.JaxrsContextPath;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
Expand Down Expand Up @@ -69,7 +70,7 @@ public void updateSpanNames(
} else {
ServerSpanNaming.updateServerSpanName(
context, ServerSpanNaming.Source.CONTROLLER, spanNameSupplier);
updateSpanName(span, spanNameForMethod(target, method));
updateSpanName(span, SpanNames.fromMethod(target, method));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.lang.reflect.Method;

Expand All @@ -28,7 +29,7 @@ protected String getInstrumentationName() {
}

public Context startSpan(Class<?> target, Method method) {
String spanName = spanNameForMethod(target, method);
String spanName = SpanNames.fromMethod(target, method);

Context parentContext = Context.current();
Span serverSpan = ServerSpan.fromContextOrNull(parentContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public final class MethodInstrumenters {
private static final Instrumenter<Method, Void> INSTRUMENTER;

static {
SpanNameExtractor<Method> spanName = SpanNames::spanNameForMethod;
SpanNameExtractor<Method> spanName = SpanNames::fromMethod;

INSTRUMENTER =
Instrumenter.<Method, Void>newBuilder(
Expand Down
Loading