Split out methods and opentelemetry-annotations modules (#1787)

This commit is contained in:
Trask Stalnaker 2020-11-28 12:25:09 -08:00 committed by GitHub
parent 2b2ffa87ed
commit f2bb2f3e30
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 164 additions and 107 deletions

View File

@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.traceannotation;
package io.opentelemetry.javaagent.instrumentation.extannotations;
import static io.opentelemetry.javaagent.instrumentation.traceannotation.TraceAnnotationTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.extannotations.TraceAnnotationTracer.tracer;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Scope;

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.traceannotation;
package io.opentelemetry.javaagent.instrumentation.extannotations;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
@ -16,6 +16,6 @@ public class TraceAnnotationTracer extends BaseTracer {
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.external-annotation";
return "io.opentelemetry.javaagent.external-annotations";
}
}

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.traceannotation;
package io.opentelemetry.javaagent.instrumentation.extannotations;
import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
@ -62,12 +62,13 @@ public class TraceAnnotationsInstrumentationModule extends InstrumentationModule
"org.springframework.cloud.sleuth.annotation.NewSpan"
};
private static final String TRACE_ANNOTATIONS_CONFIG = "otel.trace.annotations";
private static final String TRACE_ANNOTATIONS_CONFIG =
"otel.instrumentation.external-annotations.include";
private static final String TRACE_ANNOTATED_METHODS_EXCLUDE_CONFIG =
"otel.trace.annotated.methods.exclude";
"otel.instrumentation.external-annotations.exclude-methods";
public TraceAnnotationsInstrumentationModule() {
super("trace", "trace-annotation");
super("external-annotations");
}
@Override
@ -83,7 +84,7 @@ public class TraceAnnotationsInstrumentationModule extends InstrumentationModule
private final ElementMatcher.Junction<MethodDescription> excludedMethodsMatcher;
public AnnotatedMethodsInstrumentation() {
additionalTraceAnnotations = configureAdditionalTraceAnnotations();
additionalTraceAnnotations = configureAdditionalTraceAnnotations(Config.get());
if (additionalTraceAnnotations.isEmpty()) {
classLoaderOptimization = none();
@ -124,8 +125,8 @@ public class TraceAnnotationsInstrumentationModule extends InstrumentationModule
TraceAdvice.class.getName());
}
private static Set<String> configureAdditionalTraceAnnotations() {
String configString = Config.get().getProperty(TRACE_ANNOTATIONS_CONFIG);
private static Set<String> configureAdditionalTraceAnnotations(Config config) {
String configString = config.getProperty(TRACE_ANNOTATIONS_CONFIG);
if (configString == null) {
return Collections.unmodifiableSet(Sets.newHashSet(DEFAULT_ANNOTATIONS));
} else if (configString.isEmpty()) {

View File

@ -3,17 +3,15 @@
* SPDX-License-Identifier: Apache-2.0
*/
import static io.opentelemetry.javaagent.instrumentation.traceannotation.TraceAnnotationsInstrumentationModule.DEFAULT_ANNOTATIONS
import io.opentelemetry.instrumentation.test.AgentTestRunner
import io.opentelemetry.instrumentation.test.utils.ConfigUtils
import io.opentelemetry.javaagent.instrumentation.traceannotation.TraceAnnotationsInstrumentationModule
import io.opentelemetry.test.annotation.SayTracedHello
import java.util.concurrent.Callable
class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
it.setProperty("otel.trace.annotations", "package.Class\$Name;${OuterClass.InterestingMethod.name}")
it.setProperty("otel.instrumentation.external-annotations.include",
"package.Class\$Name;${OuterClass.InterestingMethod.name}")
}
def specCleanup() {
@ -44,35 +42,6 @@ class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
}
}
def "test configuration #value"() {
setup:
def previousConfig = ConfigUtils.updateConfig {
if (value) {
it.setProperty("otel.trace.annotations", value)
} else {
it.remove("otel.trace.annotations")
}
}
expect:
new TraceAnnotationsInstrumentationModule.AnnotatedMethodsInstrumentation().additionalTraceAnnotations == expected.toSet()
cleanup:
ConfigUtils.setConfig(previousConfig)
where:
value | expected
null | DEFAULT_ANNOTATIONS.toList()
" " | []
"some.Invalid[]" | []
"some.package.ClassName " | ["some.package.ClassName"]
" some.package.Class\$Name" | ["some.package.Class\$Name"]
" ClassName " | ["ClassName"]
"ClassName" | ["ClassName"]
"Class\$1;Class\$2;" | ["Class\$1", "Class\$2"]
"Duplicate ;Duplicate ;Duplicate; " | ["Duplicate"]
}
static class AnnotationTracedCallable implements Callable<String> {
@OuterClass.InterestingMethod
@Override

View File

@ -0,0 +1,42 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
import static io.opentelemetry.instrumentation.api.config.Config.normalizePropertyName
import static io.opentelemetry.javaagent.instrumentation.extannotations.TraceAnnotationsInstrumentationModule.DEFAULT_ANNOTATIONS
import io.opentelemetry.instrumentation.api.config.Config
import io.opentelemetry.javaagent.instrumentation.extannotations.TraceAnnotationsInstrumentationModule
import spock.lang.Specification
class IncludeTest extends Specification {
def "test configuration #value"() {
setup:
Config config
if (value) {
config = Config.create([
(normalizePropertyName("otel.instrumentation.external-annotations.include")): value
])
} else {
config = Config.DEFAULT
}
expect:
TraceAnnotationsInstrumentationModule.AnnotatedMethodsInstrumentation.configureAdditionalTraceAnnotations(config) == expected.toSet()
where:
value | expected
null | DEFAULT_ANNOTATIONS.toList()
" " | []
"some.Invalid[]" | []
"some.package.ClassName " | ["some.package.ClassName"]
" some.package.Class\$Name" | ["some.package.Class\$Name"]
" ClassName " | ["ClassName"]
"ClassName" | ["ClassName"]
"Class\$1;Class\$2;" | ["Class\$1", "Class\$2"]
"Duplicate ;Duplicate ;Duplicate; " | ["Duplicate"]
}
}

View File

@ -9,8 +9,8 @@ import io.opentracing.contrib.dropwizard.Trace
class TracedMethodsExclusionTest extends AgentTestRunner {
static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
it.setProperty("otel.trace.methods", "${TestClass.name}[included,excluded]")
it.setProperty("otel.trace.annotated.methods.exclude", "${TestClass.name}[excluded,annotatedButExcluded]")
it.setProperty("otel.instrumentation.external-annotations.exclude-methods",
"${TestClass.name}[excluded,annotatedButExcluded]")
}
def cleanupSpec() {
@ -18,11 +18,6 @@ class TracedMethodsExclusionTest extends AgentTestRunner {
}
static class TestClass {
//This method is configured to be traced
String included() {
return "Hello!"
}
//This method is not mentioned in any configuration
String notMentioned() {
return "Hello!"
@ -51,19 +46,11 @@ class TracedMethodsExclusionTest extends AgentTestRunner {
//Baseline and assumption validation
def "Calling these methods should be traced"() {
expect:
new TestClass().included() == "Hello!"
new TestClass().annotated() == "Hello!"
and:
assertTraces(2) {
assertTraces(1) {
trace(0, 1) {
span(0) {
name "TestClass.included"
attributes {
}
}
}
trace(1, 1) {
span(0) {
name "TestClass.annotated"
attributes {
@ -82,7 +69,7 @@ class TracedMethodsExclusionTest extends AgentTestRunner {
assertTraces(0) {}
}
def "Method which is both included and excluded for tracing should NOT be traced"() {
def "Method which is both annotated and excluded for tracing should NOT be traced"() {
expect:
new TestClass().excluded() == "Hello!"

View File

@ -0,0 +1,7 @@
apply from: "$rootDir/gradle/instrumentation.gradle"
muzzle {
pass {
coreJdk = true
}
}

View File

@ -0,0 +1,38 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.methods;
import static io.opentelemetry.javaagent.instrumentation.methods.MethodTracer.tracer;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Scope;
import java.lang.reflect.Method;
import net.bytebuddy.asm.Advice;
public class MethodAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Origin Method method,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {
span = tracer().startSpan(method);
scope = span.makeCurrent();
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) {
scope.close();
if (throwable != null) {
tracer().endExceptionally(span, throwable);
} else {
tracer().end(span);
}
}
}

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.traceannotation;
package io.opentelemetry.javaagent.instrumentation.methods;
import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
@ -33,30 +33,18 @@ import net.bytebuddy.matcher.ElementMatcher;
* super class.
*/
@AutoService(InstrumentationModule.class)
public class TraceConfigInstrumentationModule extends InstrumentationModule {
public class MethodInstrumentationModule extends InstrumentationModule {
private static final String TRACE_METHODS_CONFIG = "otel.trace.methods";
private static final String TRACE_ANNOTATED_METHODS_EXCLUDE_CONFIG =
"otel.trace.annotated.methods.exclude";
private static final String TRACE_METHODS_CONFIG = "otel.instrumentation.methods.include";
private final List<TypeInstrumentation> typeInstrumentations;
public TraceConfigInstrumentationModule() {
super("trace", "trace-config");
public MethodInstrumentationModule() {
super("methods");
Map<String, Set<String>> classMethodsToTrace =
MethodsConfigurationParser.parse(Config.get().getProperty(TRACE_METHODS_CONFIG));
Map<String, Set<String>> excludedMethods =
MethodsConfigurationParser.parse(
Config.get().getProperty(TRACE_ANNOTATED_METHODS_EXCLUDE_CONFIG));
for (Map.Entry<String, Set<String>> entry : excludedMethods.entrySet()) {
Set<String> tracedMethods = classMethodsToTrace.get(entry.getKey());
if (tracedMethods != null) {
tracedMethods.removeAll(entry.getValue());
}
}
typeInstrumentations =
classMethodsToTrace.entrySet().stream()
.filter(e -> !e.getValue().isEmpty())
@ -99,7 +87,7 @@ public class TraceConfigInstrumentationModule extends InstrumentationModule {
}
}
return Collections.singletonMap(methodMatchers, TraceAdvice.class.getName());
return Collections.singletonMap(methodMatchers, MethodAdvice.class.getName());
}
}
}

View File

@ -0,0 +1,21 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.methods;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
public class MethodTracer extends BaseTracer {
private static final MethodTracer TRACER = new MethodTracer();
public static MethodTracer tracer() {
return TRACER;
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.external-annotations";
}
}

View File

@ -7,9 +7,10 @@ import io.opentelemetry.instrumentation.test.AgentTestRunner
import io.opentelemetry.instrumentation.test.utils.ConfigUtils
import java.util.concurrent.Callable
class TraceConfigTest extends AgentTestRunner {
class MethodTest extends AgentTestRunner {
static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
it.setProperty("otel.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
it.setProperty("otel.instrumentation.methods.include",
"package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
}
def cleanupSpec() {

View File

@ -0,0 +1,11 @@
apply from: "$rootDir/gradle/instrumentation.gradle"
dependencies {
// this instrumentation needs to do similar shading dance as opentelemetry-api-1.0 because
// the @WithSpan annotation references the OpenTelemetry API's Span.Kind class
//
// see the comment in opentelemetry-api-1.0.gradle for more details
compileOnly project(path: ':opentelemetry-ext-annotations-shaded-for-instrumenting', configuration: 'shadow')
testImplementation project(path: ':opentelemetry-ext-annotations-shaded-for-instrumenting', configuration: 'shadow')
}

View File

@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.opentelemetryapi.anotations;
package io.opentelemetry.javaagent.instrumentation.otelannotations;
import static io.opentelemetry.javaagent.instrumentation.opentelemetryapi.anotations.TraceAnnotationTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.otelannotations.WithSpanTracer.tracer;
import application.io.opentelemetry.extension.annotations.WithSpan;
import io.opentelemetry.api.trace.Span;
@ -16,7 +16,7 @@ import net.bytebuddy.asm.Advice;
/**
* Instrumentation for methods annotated with {@link WithSpan} annotation.
*
* @see WithSpanAnnotationInstrumentationModule
* @see WithSpanInstrumentationModule
*/
public class WithSpanAdvice {

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.opentelemetryapi.anotations;
package io.opentelemetry.javaagent.instrumentation.otelannotations;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
@ -32,13 +32,13 @@ import net.bytebuddy.matcher.ElementMatchers;
/** Instrumentation for methods annotated with {@link WithSpan} annotation. */
@AutoService(InstrumentationModule.class)
public class WithSpanAnnotationInstrumentationModule extends InstrumentationModule {
public class WithSpanInstrumentationModule extends InstrumentationModule {
private static final String TRACE_ANNOTATED_METHODS_EXCLUDE_CONFIG =
"otel.trace.annotated.methods.exclude";
"otel.instrumentation.opentelemetry-annotations.exclude-methods";
public WithSpanAnnotationInstrumentationModule() {
super("with-span-annotation");
public WithSpanInstrumentationModule() {
super("opentelemetry-annotations");
}
@Override

View File

@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.opentelemetryapi.anotations;
package io.opentelemetry.javaagent.instrumentation.otelannotations;
import application.io.opentelemetry.api.trace.Span;
import application.io.opentelemetry.extension.annotations.WithSpan;
@ -13,14 +13,14 @@ import java.lang.reflect.Method;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class TraceAnnotationTracer extends BaseTracer {
private static final TraceAnnotationTracer TRACER = new TraceAnnotationTracer();
public class WithSpanTracer extends BaseTracer {
private static final WithSpanTracer TRACER = new WithSpanTracer();
public static TraceAnnotationTracer tracer() {
public static WithSpanTracer tracer() {
return TRACER;
}
private static final Logger log = LoggerFactory.getLogger(TraceAnnotationTracer.class);
private static final Logger log = LoggerFactory.getLogger(WithSpanTracer.class);
/**
* This method is used to generate an acceptable span (operation) name based on a given method
@ -52,6 +52,6 @@ public class TraceAnnotationTracer extends BaseTracer {
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.opentelemetry-api";
return "io.opentelemetry.javaagent.opentelemetry-annotations";
}
}

View File

@ -13,7 +13,8 @@ import io.opentelemetry.test.annotation.TracedWithSpan
*/
class WithSpanInstrumentationTest extends AgentTestRunner {
static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
it.setProperty("otel.trace.annotated.methods.exclude", "${TracedWithSpan.name}[ignored]")
it.setProperty("otel.instrumentation.opentelemetry-annotations.exclude-methods",
"${TracedWithSpan.name}[ignored]")
}
def cleanupSpec() {
@ -33,7 +34,6 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
hasNoParent()
errored false
attributes {
"providerAttr" "Otel"
}
}
}
@ -52,7 +52,6 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
hasNoParent()
errored false
attributes {
"providerAttr" "Otel"
}
}
}
@ -72,7 +71,6 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
hasNoParent()
errored false
attributes {
"providerAttr" "Otel"
}
}
}
@ -88,5 +86,4 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
Thread.sleep(500) // sleep a bit just to make sure no span is captured
assertTraces(0) {}
}
}

View File

@ -5,7 +5,6 @@
package io.opentelemetry.test.annotation;
import application.io.opentelemetry.api.trace.Span;
import application.io.opentelemetry.api.trace.Span.Kind;
import application.io.opentelemetry.extension.annotations.WithSpan;
@ -13,25 +12,21 @@ public class TracedWithSpan {
@WithSpan
public String otel() {
Span.current().setAttribute("providerAttr", "Otel");
return "hello!";
}
@WithSpan("manualName")
public String namedOtel() {
Span.current().setAttribute("providerAttr", "Otel");
return "hello!";
}
@WithSpan
public String ignored() {
Span.current().setAttribute("providerAttr", "Otel");
return "hello!";
}
@WithSpan(kind = Kind.PRODUCER)
public String oneOfAKind() {
Span.current().setAttribute("providerAttr", "Otel");
return "hello!";
}
}

View File

@ -20,10 +20,8 @@ dependencies {
// and in the code "io.opentelemetry.*" refers to the (shaded) OpenTelemetry API that is used by
// the agent (as those references will later be shaded)
compileOnly project(path: ':opentelemetry-api-shaded-for-instrumenting', configuration: 'shadow')
compileOnly project(path: ':opentelemetry-ext-annotations-shaded-for-instrumenting', configuration: 'shadow')
// using OpenTelemetry SDK to make sure that instrumentation doesn't cause
// OpenTelemetrySdk.getTracerProvider() to throw ClassCastException
testImplementation project(path: ':opentelemetry-sdk-shaded-for-instrumenting', configuration: 'shadow')
testImplementation project(path: ':opentelemetry-ext-annotations-shaded-for-instrumenting', configuration: 'shadow')
}

View File

@ -139,6 +139,7 @@ include ':instrumentation:log4j:log4j-2-testing'
include ':instrumentation:logback:logback-1.0:javaagent'
include ':instrumentation:logback:logback-1.0:library'
include ':instrumentation:logback:logback-1.0:testing'
include ':instrumentation:methods'
include ':instrumentation:mongo:mongo-3.1'
include ':instrumentation:mongo:mongo-3.7'
include ':instrumentation:mongo:mongo-async-3.3'
@ -149,6 +150,7 @@ include ':instrumentation:netty:netty-4.0'
include ':instrumentation:netty:netty-4.1'
include ':instrumentation:okhttp:okhttp-2.2'
include ':instrumentation:okhttp:okhttp-3.0'
include ':instrumentation:opentelemetry-annotations-1.0'
include ':instrumentation:opentelemetry-api-1.0'
include ':instrumentation:oshi:javaagent'
include ':instrumentation:oshi:library'