Skip to content

Add 1 minute fail-safe to JUL/JMX class-loading callback #8399

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 17, 2025
Merged

Add 1 minute fail-safe to JUL/JMX class-loading callback #8399

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 11 additions & 2 deletions dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,6 @@ public void run() {
if (appUsingCustomJMXBuilder) {
log.debug("Custom JMX builder detected. Delaying JMXFetch initialization.");
registerMBeanServerBuilderCallback(new StartJmxCallback(jmxStartDelay));
// one minute fail-safe in case nothing touches JMX and callback isn't triggered
scheduleJmxStart(60 + jmxStartDelay);
} else if (appUsingCustomLogManager) {
log.debug("Custom logger detected. Delaying JMXFetch initialization.");
registerLogManagerCallback(new StartJmxCallback(jmxStartDelay));
Expand Down Expand Up @@ -437,6 +435,8 @@ public static void startDatadogTracer(InitializationTelemetry initTelemetry) thr
}

private static void registerLogManagerCallback(final ClassLoadCallBack callback) {
// one minute fail-safe in case the class was unintentionally loaded during premain
AgentTaskScheduler.INSTANCE.schedule(callback, 1, TimeUnit.MINUTES);
try {
final Class<?> agentInstallerClass = AGENT_CLASSLOADER.loadClass(AGENT_INSTALLER_CLASS_NAME);
final Method registerCallbackMethod =
Expand All @@ -448,6 +448,8 @@ private static void registerLogManagerCallback(final ClassLoadCallBack callback)
}

private static void registerMBeanServerBuilderCallback(final ClassLoadCallBack callback) {
// one minute fail-safe in case the class was unintentionally loaded during premain
AgentTaskScheduler.INSTANCE.schedule(callback, 1, TimeUnit.MINUTES);
Copy link
Member

Choose a reason for hiding this comment

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

Originally scheduleJmxStart added some jitter to the task (not sure if it's required here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not required, in the original code it was just a side-effect of sharing the method. In the other path the jitter is used to reduce the chance of multiple Java processes that started at the same time from flooding the agent with connection attempts.

However using jitter here is only really appropriate if we had a fleet of services which:

  • all set a custom log-manager
  • all encountered the issue where JUL was unintentionally initialized during premain
  • all started at the exact same time

in such circumstances jitter would be useful, but the chance of such a scenario happening is low.

try {
final Class<?> agentInstallerClass = AGENT_CLASSLOADER.loadClass(AGENT_INSTALLER_CLASS_NAME);
final Method registerCallbackMethod =
Expand All @@ -459,8 +461,14 @@ private static void registerMBeanServerBuilderCallback(final ClassLoadCallBack c
}

protected abstract static class ClassLoadCallBack implements Runnable {
private final AtomicBoolean starting = new AtomicBoolean();

@Override
public void run() {
if (starting.getAndSet(true)) {
return; // someone has already called us
}

/*
* This callback is called from within bytecode transformer. This can be a problem if callback tries
* to load classes being transformed. To avoid this we start a thread here that calls the callback.
Expand Down Expand Up @@ -558,6 +566,7 @@ public void execute() {
}

private void resumeRemoteComponents() {
log.debug("Resuming remote components.");
try {
// remote components were paused for custom log-manager/jmx-builder
// add small delay before resuming remote I/O to help stabilization
Expand Down
Loading