Skip to content

Commit e4f3eae

Browse files
eyalkoreneyalkoren
authored
Adding instrumentation warmup (elastic#2368)
1 parent 864e4e2 commit e4f3eae

File tree

16 files changed

+323
-9
lines changed

16 files changed

+323
-9
lines changed

‎CHANGELOG.asciidoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ endif::[]
3535
* Fix runtime attach with some docker images - {pull}2385[#2385]
3636
* Restore dynamic capability to `log_level` config for plugin loggers - {pull}2384[#2384]
3737
* Fix slf4j-related `LinkageError` - {pull}2390[#2390]
38+
* Fix possible deadlock occurring when Byte Buddy reads System properties by warming up bytecode instrumentation code
39+
paths. The BCI warmup is on by default and may be disabled through the internal `warmup_byte_buddy` config option - {pull}2368[#2368]
3840
3941
[[release-notes-1.x]]
4042
=== Java Agent version 1.x

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bbwarmup/WarmupInstrumentation.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.bbwarmup;
20+
21+
import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
22+
import co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers;
23+
import co.elastic.apm.agent.bci.bytebuddy.SimpleMethodSignatureOffsetMappingFactory;
24+
import net.bytebuddy.asm.Advice;
25+
import net.bytebuddy.description.method.MethodDescription;
26+
import net.bytebuddy.description.type.TypeDescription;
27+
import net.bytebuddy.matcher.ElementMatcher;
28+
29+
import javax.annotation.Nullable;
30+
import java.util.Collection;
31+
import java.util.Collections;
32+
33+
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
34+
import static net.bytebuddy.matcher.ElementMatchers.named;
35+
import static net.bytebuddy.matcher.ElementMatchers.not;
36+
import static net.bytebuddy.matcher.ElementMatchers.returns;
37+
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;
38+
39+
public class WarmupInstrumentation extends TracerAwareInstrumentation {
40+
41+
@Override
42+
public ElementMatcher.Junction<ClassLoader> getClassLoaderMatcher() {
43+
// Instrumenting the transformed class loaded by the net.bytebuddy.dynamic.loading.ByteArrayClassLoader.ChildFirst
44+
// class loader in co.elastic.apm.agent.bci.bytebuddy.InstallationListenerImpl may produce java.lang.instrument.UnmodifiableClassException
45+
// (caused by java.lang.ClassFormatError) on OpenJDK 7.
46+
// By allowing instrumentation only when the test class is loaded by the same class loader that loads this
47+
// instrumentation class, we avoid this problem and still allow it to work both on production and unit tests
48+
return CustomElementMatchers.isSameClassLoader(getClass().getClassLoader());
49+
}
50+
51+
@Override
52+
public ElementMatcher<? super TypeDescription> getTypeMatcher() {
53+
return named("co.elastic.apm.agent.bci.bytebuddy.Instrumented");
54+
}
55+
56+
@Override
57+
public ElementMatcher<? super MethodDescription> getMethodMatcher() {
58+
return not(isStatic()).and(named("isInstrumented")).and(takesNoArguments()).and(returns(boolean.class));
59+
}
60+
61+
@Override
62+
public Collection<String> getInstrumentationGroupNames() {
63+
return Collections.emptyList();
64+
}
65+
66+
@Override
67+
public boolean includeWhenInstrumentationIsDisabled() {
68+
return true;
69+
}
70+
71+
public static class AdviceClass {
72+
@Nullable
73+
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
74+
public static Object onMethodEnter(
75+
@Advice.This Object thiz,
76+
@SimpleMethodSignatureOffsetMappingFactory.SimpleMethodSignature String signature) {
77+
return null;
78+
}
79+
80+
@Advice.AssignReturned.ToReturned
81+
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
82+
public static boolean onMethodExit(@Advice.Enter @Nullable Object nullFromEnter,
83+
@Advice.Thrown @Nullable Throwable t) {
84+
return true;
85+
}
86+
}
87+
}

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bbwarmup/package-info.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
@NonnullApi
20+
package co.elastic.apm.agent.bbwarmup;
21+
22+
import co.elastic.apm.agent.sdk.NonnullApi;

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@
2121
import co.elastic.apm.agent.bci.bytebuddy.AnnotationValueOffsetMappingFactory;
2222
import co.elastic.apm.agent.bci.bytebuddy.ErrorLoggingListener;
2323
import co.elastic.apm.agent.bci.bytebuddy.FailSafeDeclaredMethodsCompiler;
24+
import co.elastic.apm.agent.bci.bytebuddy.InstallationListenerImpl;
25+
import co.elastic.apm.agent.bci.bytebuddy.Instrumented;
2426
import co.elastic.apm.agent.bci.bytebuddy.LruTypePoolCache;
2527
import co.elastic.apm.agent.bci.bytebuddy.MatcherTimer;
2628
import co.elastic.apm.agent.bci.bytebuddy.MinimumClassFileVersionValidator;
29+
import co.elastic.apm.agent.bci.bytebuddy.NonInstrumented;
2730
import co.elastic.apm.agent.bci.bytebuddy.PatchBytecodeVersionTo51Transformer;
2831
import co.elastic.apm.agent.bci.bytebuddy.RootPackageCustomLocator;
2932
import co.elastic.apm.agent.bci.bytebuddy.SimpleMethodSignatureOffsetMappingFactory;
@@ -247,7 +250,7 @@ private static synchronized void initInstrumentation(final ElasticApmTracer trac
247250
}
248251
}
249252
for (ElasticApmInstrumentation apmInstrumentation : instrumentations) {
250-
adviceClassName2instrumentationClassLoader.put(
253+
mapInstrumentationCL2adviceClassName(
251254
apmInstrumentation.getAdviceClassName(),
252255
apmInstrumentation.getClass().getClassLoader());
253256
}
@@ -266,6 +269,14 @@ public void run() {
266269
}
267270
// POOL_ONLY because we don't want to cause eager linking on startup as the class path may not be complete yet
268271
AgentBuilder agentBuilder = initAgentBuilder(tracer, instrumentation, instrumentations, logger, AgentBuilder.DescriptionStrategy.Default.POOL_ONLY, premain);
272+
273+
// Warmup Byte Buddy and agent's invokedynamic linkage paths on the attaching thread before installing it
274+
if (tracer.getConfig(CoreConfiguration.class).shouldWarmupByteBuddy()) {
275+
agentBuilder = agentBuilder.with(new InstallationListenerImpl())
276+
.warmUp(NonInstrumented.class)
277+
.warmUp(Instrumented.class);
278+
}
279+
269280
resettableClassFileTransformer = agentBuilder.installOn(ElasticApmAgent.instrumentation);
270281
for (ConfigurationOption<?> instrumentationOption : coreConfig.getInstrumentationOptions()) {
271282
//noinspection Convert2Lambda
@@ -673,7 +684,6 @@ public Iterable<? extends List<Class<?>>> onError(int index, List<Class<?>> batc
673684
: AgentBuilder.PoolStrategy.Default.FAST)
674685
.ignore(any(), isReflectionClassLoader())
675686
.or(any(), classLoaderWithName("org.codehaus.groovy.runtime.callsite.CallSiteClassLoader"))
676-
.or(nameStartsWith("co.elastic.apm.agent.shaded"))
677687
.or(nameStartsWith("org.aspectj."))
678688
.or(nameStartsWith("org.groovy."))
679689
.or(nameStartsWith("com.p6spy."))
@@ -756,7 +766,7 @@ public static void ensureInstrumented(Class<?> classToInstrument, Collection<Cla
756766
);
757767
for (Class<? extends ElasticApmInstrumentation> instrumentationClass : instrumentationClasses) {
758768
ElasticApmInstrumentation apmInstrumentation = instantiate(instrumentationClass);
759-
adviceClassName2instrumentationClassLoader.put(
769+
mapInstrumentationCL2adviceClassName(
760770
apmInstrumentation.getAdviceClassName(),
761771
instrumentationClass.getClassLoader());
762772
ElementMatcher.Junction<? super TypeDescription> typeMatcher = getTypeMatcher(classToInstrument, apmInstrumentation.getMethodMatcher(), none());
@@ -770,6 +780,10 @@ public static void ensureInstrumented(Class<?> classToInstrument, Collection<Cla
770780
}
771781
}
772782

783+
public static void mapInstrumentationCL2adviceClassName(String adviceClassName, ClassLoader instrumentationClassLoader) {
784+
adviceClassName2instrumentationClassLoader.put(adviceClassName, instrumentationClassLoader);
785+
}
786+
773787
private static Set<Collection<Class<? extends ElasticApmInstrumentation>>> getOrCreate(Class<?> classToInstrument) {
774788
Set<Collection<Class<? extends ElasticApmInstrumentation>>> instrumentedClasses = dynamicallyInstrumentedClasses.get(classToInstrument);
775789
if (instrumentedClasses == null) {

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ static void setJavaBaseModule(Class<?> targetClass) throws Throwable {
341341
* @param adviceMethodType A {@link java.lang.invoke.MethodType} representing the arguments and return type of the advice method.
342342
* @param args Additional arguments that are provided by Byte Buddy:
343343
* <ul>
344-
* <li>A {@link String} of the binary target class name.</li>
344+
* <li>A {@link String} of the binary advice class name.</li>
345345
* <li>A {@link int} with value {@code 0} for an enter advice and {code 1} for an exist advice.</li>
346346
* <li>A {@link Class} representing the class implementing the instrumented method.</li>
347347
* <li>A {@link String} with the name of the instrumented method.</li>

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,21 @@ public static ElementMatcher.Junction<NamedElement> isInAnyPackage(Collection<St
7070
return matcher;
7171
}
7272

73+
/**
74+
* Matches the target class loader to a given class loader by instance comparison
75+
* @param other the class loader to match to
76+
* @return {@code true} if {@code other} is the same class loader instance as the target class loader
77+
*/
78+
public static ElementMatcher.Junction<ClassLoader> isSameClassLoader(final ClassLoader other) {
79+
return new ElementMatcher.Junction.AbstractBase<ClassLoader>() {
80+
@Override
81+
public boolean matches(@Nullable ClassLoader target) {
82+
return target == other;
83+
}
84+
};
85+
}
86+
87+
7388
/**
7489
* Matches only class loaders which can load a certain class.
7590
* <p>

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/InstallationListenerImpl.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.bci.bytebuddy;
20+
21+
import net.bytebuddy.agent.builder.AgentBuilder;
22+
import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
23+
import net.bytebuddy.dynamic.loading.ByteArrayClassLoader;
24+
import org.slf4j.Logger;
25+
import org.slf4j.LoggerFactory;
26+
27+
import java.lang.reflect.Method;
28+
import java.util.HashMap;
29+
import java.util.Map;
30+
31+
public class InstallationListenerImpl extends AgentBuilder.InstallationListener.Adapter {
32+
33+
@Override
34+
public void onAfterWarmUp(Map<Class<?>, byte[]> types, ResettableClassFileTransformer classFileTransformer, boolean transformed) {
35+
Logger logger = null;
36+
try {
37+
// should be safe to get a logger by now
38+
logger = LoggerFactory.getLogger(InstallationListenerImpl.class);
39+
if (!transformed) {
40+
logger.warn("Byte Buddy warmup ended without transforming at least one class. The agent may not work as expected.");
41+
} else {
42+
byte[] transformedInstrumentedClass = null;
43+
for (Map.Entry<Class<?>, byte[]> classEntry : types.entrySet()) {
44+
if (classEntry.getKey().equals(Instrumented.class)) {
45+
transformedInstrumentedClass = classEntry.getValue();
46+
break;
47+
}
48+
}
49+
50+
String instrumentedClassName = Instrumented.class.getName();
51+
if (transformedInstrumentedClass != null) {
52+
logger.debug("Warmup: bytecode transformation of {} succeeded", instrumentedClassName);
53+
54+
// Warm up the complete invokedynamic linkage route.
55+
// Byte Buddy's warmup doesn't retransform the class, it only applies the bytecode manipulation.
56+
// Therefore, we cannot warm up fully simply by invoking the instrumented method. Instead, we need
57+
// to load the instrumented class, instantiate it and invoke through reflection.
58+
HashMap<String, byte[]> typeDefinitions = new HashMap<>();
59+
typeDefinitions.put(instrumentedClassName, transformedInstrumentedClass);
60+
ByteArrayClassLoader.ChildFirst childFirstCL = new ByteArrayClassLoader.ChildFirst(null, typeDefinitions);
61+
Class<?> instrumentedClass = childFirstCL.loadClass(instrumentedClassName);
62+
Object instrumentedClassInstance = instrumentedClass.getDeclaredConstructor().newInstance();
63+
Method isInstrumentedMethod = instrumentedClass.getDeclaredMethod("isInstrumented");
64+
// this method invocation kicks the invokedynamic linkage procedure, otherwise it would result with `false`
65+
boolean isInstrumented = (boolean) isInstrumentedMethod.invoke(instrumentedClassInstance);
66+
if (isInstrumented) {
67+
Instrumented.setWarmedUp();
68+
logger.debug("Warmup: instrumented bytecode of {} was executed as expected", instrumentedClassName);
69+
} else {
70+
logger.warn("Warmup: instrumented bytecode of {} does not work as expected", instrumentedClassName);
71+
}
72+
} else {
73+
logger.warn("Warmup did not include the {} class as expected", instrumentedClassName);
74+
}
75+
}
76+
} catch (Throwable throwable) {
77+
if (logger != null) {
78+
logger.error("Unexpected bytecode instrumentation warmup error", throwable);
79+
}
80+
}
81+
}
82+
83+
@Override
84+
public void onWarmUpError(Class<?> type, ResettableClassFileTransformer classFileTransformer, Throwable throwable) {
85+
try {
86+
LoggerFactory.getLogger(InstallationListenerImpl.class).error("Error during warmup instrumentation of " + type.getName(), throwable);
87+
} catch (Throwable loggingError) {
88+
// error while trying to log
89+
}
90+
}
91+
}

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/Instrumented.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.bci.bytebuddy;
20+
21+
public class Instrumented {
22+
23+
private static volatile boolean warmedUp = false;
24+
25+
public static boolean isWarmedUp() {
26+
return warmedUp;
27+
}
28+
29+
public static void setWarmedUp() {
30+
warmedUp = true;
31+
}
32+
33+
public boolean isInstrumented() {
34+
return false;
35+
}
36+
}

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/NonInstrumented.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.bci.bytebuddy;
20+
21+
public class NonInstrumented {
22+
}

‎apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,14 @@ public String toString(Collection<String> value) {
379379
.description("When enabled, configures Byte Buddy to use a type pool cache.")
380380
.buildWithDefault(true);
381381

382+
private final ConfigurationOption<Boolean> warmupByteBuddy = ConfigurationOption.booleanOption()
383+
.key("warmup_byte_buddy")
384+
.configurationCategory(CORE_CATEGORY)
385+
.tags("internal")
386+
.description("When set to true, configures Byte Buddy to warmup instrumentation processes on the \n" +
387+
"attaching thread just before installing the transformer on the JVM Instrumentation.")
388+
.buildWithDefault(true);
389+
382390
private final ConfigurationOption<String> bytecodeDumpPath = ConfigurationOption.stringOption()
383391
.key("bytecode_dump_path")
384392
.configurationCategory(CORE_CATEGORY)
@@ -776,6 +784,10 @@ public boolean isTypePoolCacheEnabled() {
776784
return typePoolCache.get();
777785
}
778786

787+
public boolean shouldWarmupByteBuddy() {
788+
return warmupByteBuddy.get();
789+
}
790+
779791
@Nullable
780792
public String getBytecodeDumpPath() {
781793
return bytecodeDumpPath.get();

0 commit comments

Comments
 (0)