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

Introduce marker mechanism for eagerly initializing helpers #8028

Merged
merged 1 commit into from
Nov 28, 2024
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 @@ -2,6 +2,7 @@

import static java.util.Collections.singletonList;

import datadog.trace.api.Platform;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.util.List;
import java.util.concurrent.CompletableFuture;
Expand All @@ -22,6 +23,15 @@ public final class AsyncResultExtensions {
*/
public static void register(AsyncResultExtension extension) {
if (extension != null) {
if (Platform.isNativeImageBuilder()
&& extension
.getClass()
.getClassLoader()
.getClass()
.getName()
.endsWith("ThrowawayClassLoader")) {
return; // spring-native expects this to be thrown away, not persisted
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the story behind that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

spring-native uses this special class-loader to perform its own analysis during native-image generation - classes loaded by it are not supposed to be recorded as "build-time". Since AsyncResultExtensions and anything it holds will be marked as "build-time" we must not keep hold of any throw-away classes here.

https://github.com/spring-projects/spring-framework/blob/main/spring-core/src/main/java/org/springframework/aot/nativex/feature/ThrowawayClassLoader.java#L25

}
EXTENSIONS.add(extension);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.AgentClassLoading.INJECTING_HELPERS;

import datadog.trace.bootstrap.instrumentation.api.EagerHelper;
import java.io.IOException;
import java.lang.ref.WeakReference;
import java.security.CodeSource;
Expand Down Expand Up @@ -124,6 +125,18 @@ public DynamicType.Builder<?> transform(
final JavaModule javaModule = JavaModule.ofType(classes.values().iterator().next());
helperModules.add(new WeakReference<>(javaModule.unwrap()));
}

// forcibly initialize any eager helpers
for (Class<?> clazz : classes.values()) {
if (EagerHelper.class.isAssignableFrom(clazz)) {
try {
clazz.getMethod("init").invoke(null);
} catch (Throwable e) {
log.debug("Problem initializing {}", clazz, e);
}
}
}

} catch (final Exception e) {
if (log.isErrorEnabled()) {
log.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[
+ "datadog.trace.bootstrap.benchmark.StaticEventLogger:build_time,"
+ "datadog.trace.bootstrap.blocking.BlockingExceptionHandler:build_time,"
+ "datadog.trace.bootstrap.InstrumentationErrors:build_time,"
+ "datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions:rerun,"
+ "datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions:build_time,"
+ "datadog.trace.bootstrap.instrumentation.java.concurrent.ConcurrentState:build_time,"
+ "datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter:build_time,"
+ "datadog.trace.bootstrap.instrumentation.java.concurrent.QueueTimeHelper:build_time,"
Expand All @@ -105,6 +105,10 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[
+ "datadog.trace.bootstrap.instrumentation.jfr.exceptions.ExceptionSampleEvent:build_time,"
+ "datadog.trace.bootstrap.instrumentation.jfr.backpressure.BackpressureSampleEvent:build_time,"
+ "datadog.trace.bootstrap.instrumentation.jfr.directallocation.DirectAllocationTotalEvent:build_time,"
+ "datadog.trace.instrumentation.guava10.GuavaAsyncResultExtension:build_time,"
+ "datadog.trace.instrumentation.reactivestreams.ReactiveStreamsAsyncResultExtension:build_time,"
+ "datadog.trace.instrumentation.reactor.core.ReactorAsyncResultExtension:build_time,"
+ "datadog.trace.instrumentation.rxjava2.RxJavaAsyncResultExtension:build_time,"
+ "datadog.trace.logging.LoggingSettingsDescription:build_time,"
+ "datadog.trace.logging.simplelogger.SLCompatFactory:build_time,"
+ "datadog.trace.logging.LogReporter:build_time,"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

import com.google.common.util.concurrent.ListenableFuture;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.EagerHelper;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtension;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;

public class GuavaAsyncResultExtension implements AsyncResultExtension {
public class GuavaAsyncResultExtension implements AsyncResultExtension, EagerHelper {
static {
AsyncResultExtensions.register(new GuavaAsyncResultExtension());
}
Expand All @@ -19,7 +20,7 @@ public class GuavaAsyncResultExtension implements AsyncResultExtension {
* class initialization. This will ensure this extension will only be registered once under {@link
* AsyncResultExtensions}.
*/
public static void initialize() {}
public static void init() {}

@Override
public boolean supports(Class<?> result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import com.google.auto.service.AutoService;
Expand Down Expand Up @@ -47,20 +46,11 @@ public Map<String, String> contextStore() {

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
isConstructor(), ListenableFutureInstrumentation.class.getName() + "$AbstractFutureAdvice");
transformer.applyAdvice(
named("addListener").and(takesArguments(Runnable.class, Executor.class)),
ListenableFutureInstrumentation.class.getName() + "$AddListenerAdvice");
}

public static class AbstractFutureAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void init() {
GuavaAsyncResultExtension.initialize();
}
}

public static class AddListenerAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static State addListenerEnter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
import static net.bytebuddy.matcher.ElementMatchers.not;
Expand Down Expand Up @@ -69,7 +68,6 @@ public String[] helperClassNames() {

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(isConstructor(), this.getClass().getName() + "$PublisherAdvice");
transformer.applyAdvice(
isMethod()
.and(not(isStatic()))
Expand All @@ -79,13 +77,6 @@ public void methodAdvice(MethodTransformer transformer) {
getClass().getName() + "$PublisherSubscribeAdvice");
}

public static class PublisherAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void init() {
ReactiveStreamsAsyncResultExtension.initialize();
}
}

public static class PublisherSubscribeAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static AgentScope onSubscribe(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package datadog.trace.instrumentation.reactivestreams;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.EagerHelper;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtension;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions;
import org.reactivestreams.Publisher;
import org.reactivestreams.Subscriber;
import org.reactivestreams.Subscription;

public class ReactiveStreamsAsyncResultExtension implements AsyncResultExtension {
public class ReactiveStreamsAsyncResultExtension implements AsyncResultExtension, EagerHelper {
static {
AsyncResultExtensions.register(new ReactiveStreamsAsyncResultExtension());
}
Expand All @@ -19,7 +20,7 @@ public class ReactiveStreamsAsyncResultExtension implements AsyncResultExtension
* class initialization. This will ensure this extension will only be registered once under {@link
* AsyncResultExtensions}.
*/
public static void initialize() {}
public static void init() {}

@Override
public boolean supports(Class<?> result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.nameStartsWith;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;

import com.google.auto.service.AutoService;
Expand Down Expand Up @@ -40,13 +39,6 @@ public String[] helperClassNames() {
};
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(isConstructor(), getClass().getName() + "$AsyncExtensionInstallAdvice");
transformer.applyAdvice(
isMethod().and(nameStartsWith("block")), getClass().getName() + "$BlockingAdvice");
}

@Override
public Map<String, String> contextStore() {
return Collections.singletonMap("org.reactivestreams.Publisher", AgentSpan.class.getName());
Expand All @@ -62,6 +54,12 @@ public ElementMatcher<TypeDescription> hierarchyMatcher() {
return hasSuperType(namedOneOf("reactor.core.publisher.Mono", "reactor.core.publisher.Flux"));
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
isMethod().and(nameStartsWith("block")), getClass().getName() + "$BlockingAdvice");
}

public static class BlockingAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static AgentScope before(@Advice.This final Publisher self) {
Expand All @@ -79,11 +77,4 @@ public static void after(@Advice.Enter final AgentScope scope) {
}
}
}

public static class AsyncExtensionInstallAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void init() {
ReactorAsyncResultExtension.initialize();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package datadog.trace.instrumentation.reactor.core;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.EagerHelper;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtension;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

public class ReactorAsyncResultExtension implements AsyncResultExtension {
public class ReactorAsyncResultExtension implements AsyncResultExtension, EagerHelper {
static {
AsyncResultExtensions.register(new ReactorAsyncResultExtension());
}
Expand All @@ -18,7 +19,7 @@ public class ReactorAsyncResultExtension implements AsyncResultExtension {
* class initialization. This will ensure this extension will only be registered once under {@link
* AsyncResultExtensions}.
*/
public static void initialize() {}
public static void init() {}

@Override
public boolean supports(Class<?> result) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.instrumentation.rxjava2;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.EagerHelper;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtension;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AsyncResultExtensions;
import io.reactivex.Completable;
Expand All @@ -9,7 +10,7 @@
import io.reactivex.Observable;
import io.reactivex.Single;

public class RxJavaAsyncResultExtension implements AsyncResultExtension {
public class RxJavaAsyncResultExtension implements AsyncResultExtension, EagerHelper {
static {
AsyncResultExtensions.register(new RxJavaAsyncResultExtension());
}
Expand All @@ -21,7 +22,7 @@ public class RxJavaAsyncResultExtension implements AsyncResultExtension {
* class initialization. This will ensure this extension will only be registered once under {@link
* AsyncResultExtensions}.
*/
public static void initialize() {}
public static void init() {}

@Override
public boolean supports(Class<?> result) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package datadog.trace.instrumentation.rxjava2;

import static net.bytebuddy.matcher.ElementMatchers.isMethod;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.InstrumenterConfig;
import net.bytebuddy.asm.Advice;

@AutoService(InstrumenterModule.class)
public class RxJavaPluginsInstrumentation extends InstrumenterModule.Tracing
Expand Down Expand Up @@ -36,13 +33,6 @@ public String[] helperClassNames() {

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(isMethod(), getClass().getName() + "$RxJavaPluginsAdvice");
}

public static class RxJavaPluginsAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void init() {
RxJavaAsyncResultExtension.initialize();
}
// no-op
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package datadog.trace.bootstrap.instrumentation.api;

/**
* Marker interface used to identify helpers that should be eagerly initialized when injected.
*
* <p>Eager helpers must declare a public static "init" method that takes no arguments.
*/
public interface EagerHelper {}
Loading