diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverService.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverService.java index 90aaeb980f..6a3a9fec67 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverService.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverService.java @@ -66,6 +66,7 @@ import java.security.ProtectionDomain; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -91,6 +92,22 @@ public class ClassWeaverService implements ClassMatchVisitorFactory, ContextClas private static final int PARTITIONS = 8; private static ClassNode EXTENSION_TEMPLATE; + /** + * Weave packages whose method inlining should default to normalizing operand-return stacks (see + * {@code ReturnInsnProcessor.clearReturnStacks}), since weaving Kotlin-coroutine-generated bytecode + * (e.g. invokeSuspend continuations) can otherwise throw ArrayIndexOutOfBoundsException. An explicit + * -Dnewrelic.config.class_transformer.clear_return_stacks system property still overrides this default. + */ + private static final Set CLEAR_RETURN_STACKS_DEFAULT_WEAVE_PACKAGES = new HashSet<>(Arrays.asList( + "com.newrelic.instrumentation.kotlin-coroutines-1.4", + "com.newrelic.instrumentation.kotlin-coroutines-1.7", + "com.newrelic.instrumentation.kotlin-coroutines-1.9", + "com.newrelic.instrumentation.kotlin-coroutines-suspends", + "com.newrelic.instrumentation.ktor-server-core-3.0.0", + "com.newrelic.instrumentation.ktor-server-netty-3.0.0", + "com.newrelic.instrumentation.labs.ktor-utils-3.0", + "com.newrelic.instrumentation.labs.ktor-client-core-3.0")); + static { AgentBridge.extensionHolderFactory = new ExtensionHolderFactoryImpl(); try { @@ -191,18 +208,29 @@ private WeavePackage createWeavePackage(InputStream inputStream, String source) return weavePackage; } + /** + * Whether the named weave package should default return-stack normalization on (see + * {@link #CLEAR_RETURN_STACKS_DEFAULT_WEAVE_PACKAGES}). + */ + static boolean isClearReturnStacksDefaultWeavePackage(String weavePackageName) { + return CLEAR_RETURN_STACKS_DEFAULT_WEAVE_PACKAGES.contains(weavePackageName); + } + private WeavePackageConfig createWeavePackageConfig(JarInputStream jarStream, String source, Instrumentation instrumentation, WeavePackageType type, AgentConfig agentConfig) throws Exception { AgentPreprocessors preprocessors = new AgentPreprocessors(agentConfig, tracedWeaveInstrumentationDetails); AgentPostprocessors postprocessors = new AgentPostprocessors(); - WeavePackageConfig result = WeavePackageConfig.builder() + WeavePackageConfig.Builder builder = WeavePackageConfig.builder() .source(source) .jarInputStream(jarStream) .weavePreprocessor(preprocessors) .weavePostprocessor(postprocessors) .errorHandleClassNode(LogAndReturnOriginal.ERROR_HANDLER_NODE) - .extensionClassTemplate(EXTENSION_TEMPLATE) + .extensionClassTemplate(EXTENSION_TEMPLATE); + + WeavePackageConfig result = builder + .clearReturnStacksDefault(isClearReturnStacksDefaultWeavePackage(builder.getName())) .build(); preprocessors.setInstrumentationTitle(result.getName()); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverServiceTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverServiceTest.java index b0d5b32065..c7f4b35ec3 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverServiceTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/weaver/ClassWeaverServiceTest.java @@ -117,9 +117,34 @@ public void test_registerInstrumentation_validInstrumentationJar() throws Except Assert.assertNotNull(target.getWeavePackageManger().getWeavePackage("com.newrelic.instrumentation.jms-1.1")); Assert.assertEquals(1, target.getWeavePackageManger().getRegisteredPackages().size()); + + // jms-1.1 is not a Kotlin-coroutines/Ktor module, so it should not default clear_return_stacks on + Assert.assertFalse(target.getWeavePackageManger().getWeavePackage("com.newrelic.instrumentation.jms-1.1") + .getConfig().isClearReturnStacksDefault()); + } + } + + @Test + public void test_isClearReturnStacksDefaultWeavePackage_matchesKnownKotlinCoroutinesAndKtorModules() { + for (String weavePackageName : Arrays.asList( + "com.newrelic.instrumentation.kotlin-coroutines-1.4", + "com.newrelic.instrumentation.kotlin-coroutines-1.7", + "com.newrelic.instrumentation.kotlin-coroutines-1.9", + "com.newrelic.instrumentation.kotlin-coroutines-suspends", + "com.newrelic.instrumentation.ktor-server-core-3.0.0", + "com.newrelic.instrumentation.ktor-server-netty-3.0.0", + "com.newrelic.instrumentation.labs.ktor-utils-3.0", + "com.newrelic.instrumentation.labs.ktor-client-core-3.0")) { + Assert.assertTrue(weavePackageName, ClassWeaverService.isClearReturnStacksDefaultWeavePackage(weavePackageName)); } } + @Test + public void test_isClearReturnStacksDefaultWeavePackage_doesNotMatchOtherModules() { + Assert.assertFalse(ClassWeaverService.isClearReturnStacksDefaultWeavePackage("com.newrelic.instrumentation.jms-1.1")); + Assert.assertFalse(ClassWeaverService.isClearReturnStacksDefaultWeavePackage(null)); + } + @Test public void test_registerInstrumentation_noTitleInstrumentationJar() throws Exception { createServiceManager(null); diff --git a/newrelic-weaver/src/main/java/com/newrelic/weave/ClassWeave.java b/newrelic-weaver/src/main/java/com/newrelic/weave/ClassWeave.java index 6e93d5e1a5..80af7158b4 100644 --- a/newrelic-weaver/src/main/java/com/newrelic/weave/ClassWeave.java +++ b/newrelic-weaver/src/main/java/com/newrelic/weave/ClassWeave.java @@ -11,6 +11,7 @@ import com.newrelic.weave.utils.SynchronizedClassNode; import com.newrelic.weave.utils.WeaveUtils; import com.newrelic.weave.weavepackage.WeavePackage; +import com.newrelic.weave.weavepackage.WeavePackageConfig; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; @@ -280,8 +281,9 @@ private MethodNode weaveMethod(final MethodNode weaveMethod, MethodNode targetMe } // inline original invocation + WeavePackageConfig weavePackageConfig = weavePackage != null ? weavePackage.getConfig() : null; composite = MethodProcessors.inlineMethods(WeaveUtils.INLINER_PREFIX + weaveClassName, toInline, target.name, - composite); + composite, weavePackageConfig); // the inliner sometimes like to sneak in some jsr instructions. Sneaky inliner! composite = MethodProcessors.removeJSRInstructions(composite); diff --git a/newrelic-weaver/src/main/java/com/newrelic/weave/MethodCallInlinerAdapter.java b/newrelic-weaver/src/main/java/com/newrelic/weave/MethodCallInlinerAdapter.java index 0d3014ff22..8906db95c5 100644 --- a/newrelic-weaver/src/main/java/com/newrelic/weave/MethodCallInlinerAdapter.java +++ b/newrelic-weaver/src/main/java/com/newrelic/weave/MethodCallInlinerAdapter.java @@ -9,6 +9,7 @@ import com.newrelic.weave.utils.ReturnInsnProcessor; import com.newrelic.weave.utils.WeaveUtils; +import com.newrelic.weave.weavepackage.WeavePackageConfig; import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; @@ -28,6 +29,9 @@ import java.util.ArrayList; public abstract class MethodCallInlinerAdapter extends LocalVariablesSorter { + + private static final String CLEAR_RETURN_STACKS_PROPERTY = "newrelic.config.class_transformer.clear_return_stacks"; + /** * try/catch blocks which originated from the inlined method. */ @@ -40,6 +44,14 @@ public abstract class MethodCallInlinerAdapter extends LocalVariablesSorter { */ private final AnalyzerAdapter analyzerAdapter; + /* + * Config of the weave package whose methods are being inlined, or null if none is available (e.g. some test + * call paths). Threaded through so that per-package defaults (e.g. whether return-instruction stack + * normalization should be applied by default, see ReturnInsnProcessor.clearReturnStacks) can be read here + * without requiring a new constructor parameter for each new default this class needs to consult. + */ + private final WeavePackageConfig weavePackageConfig; + /* * If a method to be inlined is called several times from the same caller method, we don't want to allocate new * local variables for each invocation. Instead, we want to share the local slots between all call sites. To do this @@ -75,14 +87,25 @@ public InlinedMethod(final MethodNode method, final Remapper remapper) { // they must be computed from scratch in the ClassWriter. public MethodCallInlinerAdapter(String owner, int access, String name, String desc, MethodVisitor next, boolean inlineFrames) { - this(WeaveUtils.ASM_API_LEVEL, owner, access, name, desc, next, inlineFrames); + this(owner, access, name, desc, next, inlineFrames, null); + } + + public MethodCallInlinerAdapter(String owner, int access, String name, String desc, MethodVisitor next, + boolean inlineFrames, WeavePackageConfig weavePackageConfig) { + this(WeaveUtils.ASM_API_LEVEL, owner, access, name, desc, next, inlineFrames, weavePackageConfig); } protected MethodCallInlinerAdapter(int api, String owner, int access, String name, String desc, MethodVisitor next, boolean inlineFrames) { + this(api, owner, access, name, desc, next, inlineFrames, null); + } + + protected MethodCallInlinerAdapter(int api, String owner, int access, String name, String desc, MethodVisitor next, + boolean inlineFrames, WeavePackageConfig weavePackageConfig) { super(api, access, desc, getNext(owner, access, name, desc, next, inlineFrames)); this.analyzerAdapter = inlineFrames ? (AnalyzerAdapter) mv : null; this.mv = new InlinedTryCatchBlockSorter(WeaveUtils.ASM_API_LEVEL, this.mv, access, name, desc, null, null); + this.weavePackageConfig = weavePackageConfig; } private static MethodVisitor getNext(String owner, int access, String name, String desc, MethodVisitor next, @@ -135,8 +158,15 @@ private InlinedMethod getInliner(String owner, String name, String desc) { MethodNode methodNodeCopy = WeaveUtils.copy(method.method); //Feature flag for method nodes that require additional return insn processing. //Introduced because some Kotlin code throws ArrayIndexOutOfBoundsException when weaved. - //To enable this feature, set -Dnewrelic.config.class_transformer.clear_return_stacks=true - if (Boolean.getBoolean("newrelic.config.class_transformer.clear_return_stacks")) { + //Defaults on for weave packages whose config sets clearReturnStacksDefault (e.g. Kotlin-coroutines/ + //Ktor modules); an explicit -Dnewrelic.config.class_transformer.clear_return_stacks system property + //always overrides that default in either direction. + boolean clearReturnStacksDefault = weavePackageConfig != null + && weavePackageConfig.isClearReturnStacksDefault(); + String clearReturnStacksProp = System.getProperty(CLEAR_RETURN_STACKS_PROPERTY); + boolean clearReturnStacks = clearReturnStacksProp != null + ? Boolean.parseBoolean(clearReturnStacksProp) : clearReturnStacksDefault; + if (clearReturnStacks) { MethodNode result = WeaveUtils.newMethodNode(methodNodeCopy); methodNodeCopy.accept(new ClearReturnAdapter(owner, methodNodeCopy, result)); methodNodeCopy = result; diff --git a/newrelic-weaver/src/main/java/com/newrelic/weave/MethodProcessors.java b/newrelic-weaver/src/main/java/com/newrelic/weave/MethodProcessors.java index 6c128f3d31..c9e0db64db 100644 --- a/newrelic-weaver/src/main/java/com/newrelic/weave/MethodProcessors.java +++ b/newrelic-weaver/src/main/java/com/newrelic/weave/MethodProcessors.java @@ -11,6 +11,7 @@ import com.newrelic.api.agent.weaver.Weaver; import com.newrelic.weave.utils.WeaveUtils; import com.newrelic.weave.weavepackage.AnnotationProxyTemplate; +import com.newrelic.weave.weavepackage.WeavePackageConfig; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; @@ -56,11 +57,29 @@ private MethodProcessors() { */ public static MethodNode inlineMethods(final String inlineOwnerClassName, final Iterable inlineMethods, final String subjectOwnerClassName, final MethodNode subjectMethod) { + return inlineMethods(inlineOwnerClassName, inlineMethods, subjectOwnerClassName, subjectMethod, null); + } + + /** + * Inline all of the specified method calls in the subject method. + * + * @param inlineOwnerClassName owner class of the methods to inline + * @param inlineMethods methods to inline + * @param subjectOwnerClassName owner class of the subject method that will have calls inlined in + * @param subjectMethod subject method to inline calls into + * @param weavePackageConfig config of the weave package the subject method belongs to, used to look up + * per-package defaults (e.g. return-stack normalization) that apply absent an explicit system property + * override; may be null if unavailable + * @return new subject method with all specified methods inlined + */ + public static MethodNode inlineMethods(final String inlineOwnerClassName, final Iterable inlineMethods, + final String subjectOwnerClassName, final MethodNode subjectMethod, + final WeavePackageConfig weavePackageConfig) { MethodNode result = WeaveUtils.newMethodNode(subjectMethod); subjectMethod.accept(getInlineMethodsVisitor(subjectOwnerClassName, subjectMethod.access, subjectMethod.name, - subjectMethod.desc, result, inlineOwnerClassName, inlineMethods)); + subjectMethod.desc, result, inlineOwnerClassName, inlineMethods, weavePackageConfig)); return result; } @@ -79,8 +98,28 @@ public static MethodNode inlineMethods(final String inlineOwnerClassName, final */ public static MethodVisitor getInlineMethodsVisitor(String owner, int access, String name, String desc, MethodVisitor delegate, final String inlineOwnerClassName, final Iterable inlineMethods) { + return getInlineMethodsVisitor(owner, access, name, desc, delegate, inlineOwnerClassName, inlineMethods, null); + } + + /** + * Get a visitor that inlines all of the specified method calls into a subject method. + * + * @param owner subject method owner + * @param access subject method access + * @param name subject method name + * @param desc subject method desc + * @param delegate delegate to collect results + * @param inlineOwnerClassName owner of methods to inline + * @param inlineMethods methods to inline + * @param weavePackageConfig config of the weave package the subject method belongs to, used to look up + * per-package defaults that apply absent an explicit system property override; may be null if unavailable + * @return visitor that inlines all of the specified method calls into a subject method + */ + public static MethodVisitor getInlineMethodsVisitor(String owner, int access, String name, String desc, + MethodVisitor delegate, final String inlineOwnerClassName, final Iterable inlineMethods, + final WeavePackageConfig weavePackageConfig) { - return new MethodCallInlinerAdapter(owner, access, name, desc, delegate, false) { + return new MethodCallInlinerAdapter(owner, access, name, desc, delegate, false, weavePackageConfig) { @Override protected InlinedMethod mustInline(String owner, String name, String desc) { diff --git a/newrelic-weaver/src/main/java/com/newrelic/weave/weavepackage/WeavePackageConfig.java b/newrelic-weaver/src/main/java/com/newrelic/weave/weavepackage/WeavePackageConfig.java index 704c9e1e02..e43225a9f1 100644 --- a/newrelic-weaver/src/main/java/com/newrelic/weave/weavepackage/WeavePackageConfig.java +++ b/newrelic-weaver/src/main/java/com/newrelic/weave/weavepackage/WeavePackageConfig.java @@ -40,6 +40,7 @@ public static class Builder { private boolean enabled = true; private long priority = 0L; private boolean custom = false; + private boolean clearReturnStacksDefault = false; private WeaveViolationFilter weaveViolationFilter = null; private ClassNode errorHandleClassNode = ErrorTrapHandler.NO_ERROR_TRAP_HANDLER; private WeavePreprocessor preprocessor = WeavePreprocessor.NO_PREPROCESSOR; @@ -59,6 +60,17 @@ public Builder name(String name) { return this; } + /** + * The name of the weave package as set so far, or null if not yet set. Useful for making decisions (e.g. + * about other builder parameters) based on the package name before calling {@link #build()}, since + * {@link WeavePackageConfig} itself is immutable once built. + * + * @return the package name as set so far, or null + */ + public String getName() { + return name; + } + /** * Set the instance of {@link Instrumentation} to be used for weaving the bootstrap. * @@ -202,6 +214,19 @@ public Builder custom(boolean custom) { return this; } + /** + * Set whether return-instruction stack normalization (see {@code ReturnInsnProcessor.clearReturnStacks}) + * should be applied by default when weaving methods from this package, absent an explicit + * {@code -Dnewrelic.config.class_transformer.clear_return_stacks} system property override. + * + * @param clearReturnStacksDefault whether to default return-stack normalization on for this package + * @return builder with updated state + */ + public Builder clearReturnStacksDefault(boolean clearReturnStacksDefault) { + this.clearReturnStacksDefault = clearReturnStacksDefault; + return this; + } + /** * Set the {@link ClassNode} conforming to the {@link ErrorTrapHandler} specification to be used as the error * trap. @@ -331,8 +356,9 @@ public WeavePackageConfig build() { if (null == name) { throw new RuntimeException("WeavePackageConfig must have an Implementation-Name"); } - return new WeavePackageConfig(name, alias, vendorId, version, enabled, priority, source, custom, instrumentation, - errorHandleClassNode, preprocessor, postprocessor, extensionClassTemplate, weaveViolationFilter); + return new WeavePackageConfig(name, alias, vendorId, version, enabled, priority, source, custom, + clearReturnStacksDefault, instrumentation, errorHandleClassNode, preprocessor, postprocessor, + extensionClassTemplate, weaveViolationFilter); } } @@ -358,6 +384,7 @@ public static Builder builder() { private final String source; private final boolean enabled; private final boolean custom; + private final boolean clearReturnStacksDefault; private final ClassNode errorHandleClassNode; private final WeavePreprocessor preprocessor; private final WeavePostprocessor postprocessor; @@ -366,8 +393,9 @@ public static Builder builder() { private final WeaveViolationFilter weaveViolationFilter; private WeavePackageConfig(String name, String alias, String vendorId, float version, boolean enabled, - long priority, String source, boolean custom, Instrumentation instrumentation, ClassNode errorTrapClassNode, - WeavePreprocessor preprocessor, WeavePostprocessor postprocessor, ClassNode extensionClassTemplate, + long priority, String source, boolean custom, boolean clearReturnStacksDefault, + Instrumentation instrumentation, ClassNode errorTrapClassNode, WeavePreprocessor preprocessor, + WeavePostprocessor postprocessor, ClassNode extensionClassTemplate, WeaveViolationFilter weaveViolationFilter) { this.name = name; this.alias = alias; @@ -377,6 +405,7 @@ private WeavePackageConfig(String name, String alias, String vendorId, float ver this.priority = priority; this.source = source; this.custom = custom; + this.clearReturnStacksDefault = clearReturnStacksDefault; this.instrumentation = instrumentation; this.errorHandleClassNode = errorTrapClassNode; @@ -444,6 +473,15 @@ public boolean isCustom() { return custom; } + /** + * Whether return-instruction stack normalization should be applied by default when weaving methods from this + * package, absent an explicit {@code -Dnewrelic.config.class_transformer.clear_return_stacks} system property + * override. + */ + public boolean isClearReturnStacksDefault() { + return clearReturnStacksDefault; + } + /** * The {@link WeaveViolationFilter} instance, if violations to filter are configured in the module manifest; null otherwise */ diff --git a/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveCoroutineTest.java b/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveCoroutineTest.java index ef89859dc6..712bd62551 100644 --- a/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveCoroutineTest.java +++ b/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveCoroutineTest.java @@ -7,6 +7,8 @@ package com.newrelic.weave; +import com.newrelic.weave.weavepackage.WeavePackage; +import com.newrelic.weave.weavepackage.WeavePackageConfig; import com.newrelic.weave.weavepackage.testclasses.SampleCoroutineKt; import org.junit.After; import org.junit.AfterClass; @@ -19,6 +21,7 @@ import org.objectweb.asm.tree.ClassNode; import java.io.IOException; +import java.util.Collections; import java.util.List; import static org.junit.Assert.assertEquals; @@ -140,6 +143,24 @@ public void featureFlagNotSetThrowsAIOOBException() throws IOException { } + //Proves that a weave package flagged as clearReturnStacksDefault fixes the AIOOBE on its own, without + //requiring the -Dnewrelic.config.class_transformer.clear_return_stacks system property to be set. + @Test + public void moduleScopedDefaultAvoidsAIOOBExceptionWithoutFeatureFlag() throws IOException { + System.clearProperty("newrelic.config.class_transformer.clear_return_stacks"); + + WeavePackageConfig config = WeavePackageConfig.builder() + .name("com.newrelic.instrumentation.kotlin-coroutines-1.9") + .clearReturnStacksDefault(true) + .build(); + WeavePackage weavePackage = new WeavePackage(config, Collections.emptyList()); + + WeaveTestUtils.weaveAndAddToContextClassloader( + "com.newrelic.weave.weavepackage.testclasses.SampleCoroutineKt$doExpectedErrorSuspend$1$1", + "com.newrelic.weave.weavepackage.testclasses.Weave_SampleCoroutine", weavePackage); + //no exception means the per-package default alone (no -D flag) avoided the AIOOBE + } + private MethodNode getNodeNamed(List methods, String targetName) { for (MethodNode mn : methods) { if (mn.name.equals(targetName)) { diff --git a/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveTestUtils.java b/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveTestUtils.java index 82e7a11112..657c025fe6 100644 --- a/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveTestUtils.java +++ b/newrelic-weaver/src/test/java/com/newrelic/weave/WeaveTestUtils.java @@ -377,6 +377,23 @@ public static ClassWeave weaveAndAddToContextClassloader(String originalName, St return weaveAndAddToContextClassloader(originalName, weaveName, originalName); } + /** + * Same as {@link #weaveAndAddToContextClassloader(String, String)}, but weaves using the given + * {@link WeavePackage} so that per-package config (e.g. clear-return-stacks default) is honored. + */ + public static ClassWeave weaveAndAddToContextClassloader(String originalName, String weaveName, + WeavePackage weavePackage) throws IOException { + ClassNode errorHandlerClassNode = WeaveTestUtils.readClass(ERROR_TRAP); + ClassNode originalClass = readClass(originalName); + ClassNode weaveClass = readClass(weaveName); + ClassCache contextCache = createContextCache(); + ClassWeave weave = weave(originalClass, weaveClass, originalClass, false, + Collections.emptySet(), Collections.emptySet(), errorHandlerClassNode, + ExtensionClassTemplate.DEFAULT_EXTENSION_TEMPLATE, contextCache, weavePackage); + addToContextClassloader(weave, contextCache); + return weave; + } + public static ClassWeave weaveAndAddToContextClassloader(ClassNode originalClass, ClassNode weaveClass) throws IOException { ClassNode errorHandlerClassNode = WeaveTestUtils.readClass( @@ -424,6 +441,19 @@ public static ClassWeave weave(ClassNode originalClass, ClassNode weaveClass, ClassNode target, boolean isBaseMatch, Set requiredClassAnnotations, Set requiredMethodAnnotations, ClassNode errorHandlerClassNode, ClassNode extensionTemplate, ClassCache contextCache) throws IOException { + return weave(originalClass, weaveClass, target, isBaseMatch, requiredClassAnnotations, + requiredMethodAnnotations, errorHandlerClassNode, extensionTemplate, contextCache, null); + } + + /** + * Same as {@link #weave(ClassNode, ClassNode, ClassNode, boolean, Set, Set, ClassNode, ClassNode, ClassCache)}, + * but weaves using the given {@link WeavePackage} instead of no weave package (null), so that per-package + * config (e.g. {@code WeavePackageConfig#isClearReturnStacksDefault()}) is honored during weaving. + */ + public static ClassWeave weave(ClassNode originalClass, ClassNode weaveClass, + ClassNode target, boolean isBaseMatch, Set requiredClassAnnotations, + Set requiredMethodAnnotations, ClassNode errorHandlerClassNode, ClassNode extensionTemplate, + ClassCache contextCache, WeavePackage weavePackage) throws IOException { // match original and weave only if we haven't already done so PreparedMatch preparedMatch = MATCHES_ADDED_TO_CLASSLOADER.get(originalClass.name, weaveClass.name); @@ -458,7 +488,7 @@ public static ClassWeave weave(ClassNode originalClass, ClassNode weaveClass, } // weave target using the specified match and add composite class to classloader - return ClassWeave.weave(preparedMatch, target, null, Collections.emptyMap()); + return ClassWeave.weave(preparedMatch, target, weavePackage, Collections.emptyMap()); } /** diff --git a/newrelic-weaver/src/test/java/com/newrelic/weave/weavepackage/WeavePackageConfigTest.java b/newrelic-weaver/src/test/java/com/newrelic/weave/weavepackage/WeavePackageConfigTest.java index 70a5d57b59..3f3394da2c 100644 --- a/newrelic-weaver/src/test/java/com/newrelic/weave/weavepackage/WeavePackageConfigTest.java +++ b/newrelic-weaver/src/test/java/com/newrelic/weave/weavepackage/WeavePackageConfigTest.java @@ -6,6 +6,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -30,4 +31,29 @@ public void ConfigBuilder_withoutConfiguredFilters_doesNotCreatesWeaveViolationF assertNull(config.getWeaveViolationFilter()); } + + @Test + public void ConfigBuilder_clearReturnStacksDefault_defaultsToFalse() { + WeavePackageConfig config = new WeavePackageConfig.Builder().name("test").build(); + + assertFalse(config.isClearReturnStacksDefault()); + } + + @Test + public void ConfigBuilder_withClearReturnStacksDefaultSetTrue_isClearReturnStacksDefaultReturnsTrue() { + WeavePackageConfig config = new WeavePackageConfig.Builder().name("test") + .clearReturnStacksDefault(true) + .build(); + + assertTrue(config.isClearReturnStacksDefault()); + } + + @Test + public void ConfigBuilder_getName_returnsNameSetSoFar() { + WeavePackageConfig.Builder builder = new WeavePackageConfig.Builder(); + assertNull(builder.getName()); + + builder.name("test"); + assertEquals("test", builder.getName()); + } }