Instrument servlet invocations with BCI

apm-agent-benchmarks/src/main/java/co/elastic/apm/benchmark/reporter/AbstractReporterBenchmark.java

@@ -129,8 +129,8 @@ private void fillTransaction(Transaction t) {
         request.addHeader("cookie", "c1=v1; c2=v2");
         request.addHeader("some-other-header", "foo");
         request.addHeader("array", "foo, bar, baz");
-        request.getCookies().put("c1", "v1");
-        request.getCookies().put("c2", "v2");
+        request.getCookies().add("c1", "v1");
+        request.getCookies().add("c2", "v2");
 
         context.getResponse()
             .withStatusCode(200)

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

@@ -20,8 +20,11 @@
 package co.elastic.apm.bci;
 
 import co.elastic.apm.bci.bytebuddy.ErrorLoggingListener;
+import co.elastic.apm.configuration.CoreConfiguration;
+import co.elastic.apm.impl.ElasticApmTracer;
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.agent.builder.AgentBuilder;
+import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.scaffold.MethodGraph;
@@ -31,6 +34,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nullable;
 import java.lang.instrument.Instrumentation;
 import java.security.ProtectionDomain;
 import java.util.ServiceLoader;
@@ -39,10 +43,16 @@
 import static co.elastic.apm.bci.bytebuddy.ClassLoaderNameMatcher.isReflectionClassLoader;
 import static net.bytebuddy.asm.Advice.ExceptionHandler.Default.PRINTING;
 import static net.bytebuddy.matcher.ElementMatchers.any;
+import static net.bytebuddy.matcher.ElementMatchers.nameContains;
+import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
 
 public class ElasticApmAgent {
 
     private static final Logger logger = LoggerFactory.getLogger(ElasticApmAgent.class);
+    @Nullable
+    private static Instrumentation instrumentation;
+    @Nullable
+    private static ResettableClassFileTransformer resettableClassFileTransformer;
 
     /**
      * Allows the installation of this agent via the {@code javaagent} command line argument.
@@ -51,7 +61,7 @@ public class ElasticApmAgent {
      * @param instrumentation The instrumentation instance.
      */
     public static void premain(String agentArguments, Instrumentation instrumentation) {
-        initInstrumentation(instrumentation);
+        initInstrumentation(ElasticApmTracer.builder().build(), instrumentation);
     }
 
     /**
@@ -62,46 +72,84 @@ public static void premain(String agentArguments, Instrumentation instrumentatio
      */
     @SuppressWarnings("unused")
     public static void agentmain(String agentArguments, Instrumentation instrumentation) {
-        initInstrumentation(instrumentation);
+        initInstrumentation(ElasticApmTracer.builder().build(), instrumentation);
     }
 
-    static void initInstrumentation(Instrumentation instrumentation) {
+    public static void initInstrumentation(ElasticApmTracer tracer, Instrumentation instrumentation) {
+        initInstrumentation(tracer, instrumentation, ServiceLoader.load(ElasticApmInstrumentation.class, ElasticApmInstrumentation.class.getClassLoader()));
+    }
+
+    public static void initInstrumentation(ElasticApmTracer tracer, Instrumentation instrumentation,
+                                           Iterable<ElasticApmInstrumentation> instrumentations) {
+        if (!tracer.getConfig(CoreConfiguration.class).isInstrument()) {
+            logger.info("Instrumentation is disabled");
+            return;
+        }
+        if (ElasticApmAgent.instrumentation != null) {
+            logger.warn("Instrumentation has already been initialized");
+            return;
+        }
+        ElasticApmAgent.instrumentation = instrumentation;
         final ByteBuddy byteBuddy = new ByteBuddy()
-            .with(TypeValidation.of(true)) // TODO make false default to improve performance
+            .with(TypeValidation.of(logger.isDebugEnabled()))
             .with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE);
         AgentBuilder agentBuilder = getAgentBuilder(byteBuddy);
-        for (final ElasticApmAdvice advice : ServiceLoader.load(ElasticApmAdvice.class, ElasticApmAdvice.class.getClassLoader())) {
-            logger.debug("Applying advice {}", advice);
-            agentBuilder = agentBuilder
-                .type(new AgentBuilder.RawMatcher() {
+        int numberOfAdvices = 0;
+        for (final ElasticApmInstrumentation advice : instrumentations) {
+            numberOfAdvices++;
+            agentBuilder = applyAdvice(tracer, agentBuilder, advice);
+        }
+        logger.debug("Applied {} advices", numberOfAdvices);
+
+        resettableClassFileTransformer = agentBuilder.installOn(ElasticApmAgent.instrumentation);
+    }
+
+    private static AgentBuilder applyAdvice(final ElasticApmTracer tracer, final AgentBuilder agentBuilder,
+                                            final ElasticApmInstrumentation advice) {
+        logger.debug("Applying advice {}", advice.getClass().getName());
+        advice.init(tracer);
+        return agentBuilder
+            .type(new AgentBuilder.RawMatcher() {
+                @Override
+                public boolean matches(TypeDescription typeDescription, ClassLoader classLoader, JavaModule module, Class<?> classBeingRedefined, ProtectionDomain protectionDomain) {
+                    final boolean typeMatches = advice.getTypeMatcher().matches(typeDescription);
+                    if (typeMatches) {
+                        logger.debug("Type match for advice {}: {} matches {}",
+                            advice.getClass().getSimpleName(), advice.getTypeMatcher(), typeDescription);
+                    }
+                    return typeMatches;
+                }
+            })
+            .transform(new AgentBuilder.Transformer.ForAdvice()
+                .advice(new ElementMatcher<MethodDescription>() {
                     @Override
-                    public boolean matches(TypeDescription typeDescription, ClassLoader classLoader, JavaModule module, Class<?> classBeingRedefined, ProtectionDomain protectionDomain) {
-                        final boolean typeMatches = advice.getTypeMatcher().matches(typeDescription);
-                        if (typeMatches) {
-                            logger.debug("Type match for advice {}: {} matches {}",
-                                advice.getClass().getSimpleName(), advice.getTypeMatcher(), typeDescription);
+                    public boolean matches(MethodDescription target) {
+                        final boolean matches = advice.getMethodMatcher().matches(target);
+                        if (matches) {
+                            logger.debug("Method match for advice {}: {} matches {}",
+                                advice.getClass().getSimpleName(), advice.getMethodMatcher(), target);
                         }
-                        return typeMatches;
+                        return matches;
                     }
-                })
-                .transform(new AgentBuilder.Transformer.ForAdvice()
-                    .advice(new ElementMatcher<MethodDescription>() {
-                        @Override
-                        public boolean matches(MethodDescription target) {
-                            final boolean matches = advice.getMethodMatcher().matches(target);
-                            if (matches) {
-                                logger.debug("Method match for advice {}: {} matches {}",
-                                    advice.getClass().getSimpleName(), advice.getMethodMatcher(), target);
-                            }
-                            return matches;
-                        }
-                    }, advice.getClass().getName())
-                    .include(advice.getClass().getClassLoader())
-                    .withExceptionHandler(PRINTING))
-                .asDecorator();
-        }
+                }, advice.getAdviceClass().getName())
+                .include(advice.getAdviceClass().getClassLoader())
+                .withExceptionHandler(PRINTING))
+            .asDecorator();
+    }
 
-        agentBuilder.installOn(instrumentation);
+    /**
+     * Reverts instrumentation of classes and re-transforms them to their state without the agent.
+     * <p>
+     * This is only to be used for unit tests
+     * </p>
+     */
+    public static synchronized void reset() {
+        if (resettableClassFileTransformer == null || instrumentation == null) {
+            throw new IllegalStateException("Reset was called before init");
+        }
+        resettableClassFileTransformer.reset(instrumentation, AgentBuilder.RedefinitionStrategy.RETRANSFORMATION);
+        instrumentation = null;
+        resettableClassFileTransformer = null;
     }
 
     private static AgentBuilder getAgentBuilder(ByteBuddy byteBuddy) {
@@ -110,6 +158,15 @@ private static AgentBuilder getAgentBuilder(ByteBuddy byteBuddy) {
             .with(new ErrorLoggingListener())
             .ignore(any(), isReflectionClassLoader())
             .or(any(), classLoaderWithName("org.codehaus.groovy.runtime.callsite.CallSiteClassLoader"))
+            .or(nameStartsWith("java."))
+            .or(nameStartsWith("com.sun."))
+            .or(nameStartsWith("sun"))
+            .or(nameStartsWith("org.aspectj."))
+            .or(nameStartsWith("org.groovy."))
+            .or(nameStartsWith("com.p6spy."))
+            .or(nameStartsWith("net.bytebuddy."))
+            .or(nameContains("javassist"))
+            .or(nameContains(".asm."))
             .disableClassFormatChanges();
     }
 

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

@@ -0,0 +1,74 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 the original author or authors
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+package co.elastic.apm.bci;
+
+import co.elastic.apm.impl.ElasticApmTracer;
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+/**
+ * An advice is responsible for instrumenting methods (see {@link #getMethodMatcher()}) in particular classes
+ * (see {@link #getTypeMatcher()}).
+ * <p>
+ * The actual instrumentation of the matched methods is performed by static methods within this class,
+ * which are annotated by {@link net.bytebuddy.asm.Advice.OnMethodEnter} or {@link net.bytebuddy.asm.Advice.OnMethodExit}.
+ * </p>
+ * <p>
+ * Note: usage of {@link ElasticApmTracer#get()} is discouraged in advices,
+ * use the instance provided by {@link ElasticApmInstrumentation#init(ElasticApmTracer)} instead.
+ * </p>
+ */
+public abstract class ElasticApmInstrumentation {
+
+    /**
+     * Initializes the advice with the {@link ElasticApmTracer}
+     * <p>
+     * This enables tests to register a custom instance with a {@link co.elastic.apm.impl.ElasticApmTracerBuilder#configurationRegistry}
+     * and {@link co.elastic.apm.impl.ElasticApmTracerBuilder#reporter} which is specific to a particular test or test class.
+     * Otherwise, the advice would just have a static reference to {@link ElasticApmTracer#get()},
+     * without the possibility to register custom instances.
+     * </p>
+     *
+     * @param tracer the tracer to use for this advice.
+     */
+    public void init(ElasticApmTracer tracer) {
+    }
+
+    /**
+     * The type matcher selects types which should be instrumented by this advice
+     *
+     * @return the type matcher
+     */
+    public abstract ElementMatcher<? super TypeDescription> getTypeMatcher();
+
+    /**
+     * The method matcher selects methods of types matching {@link #getTypeMatcher()},
+     * which should be instrumented
+     *
+     * @return the method matcher
+     */
+    public abstract ElementMatcher<? super MethodDescription> getMethodMatcher();
+
+    public Class<?> getAdviceClass() {
+        return getClass();
+    }
+
+}

apm-agent-core/src/main/java/co/elastic/apm/bci/ElasticApmAdvice.java → apm-agent-core/src/main/java/co/elastic/apm/bci/VisibleForAdvice.java

@@ -19,14 +19,16 @@
  */
 package co.elastic.apm.bci;
 
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-public abstract class ElasticApmAdvice {
-
-    public abstract ElementMatcher<? super TypeDescription> getTypeMatcher();
-
-    public abstract ElementMatcher<? super MethodDescription> getMethodMatcher();
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 
+/**
+ * A marker annotation which indicates that the annotated field or method has to be public because it is called by advice methods,
+ * which are inlined into other classes.
+ */
+@Retention(RetentionPolicy.SOURCE)
+@Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD})
+public @interface VisibleForAdvice {
 }

apm-agent-core/src/main/java/co/elastic/apm/impl/ElasticApmTracerBuilder.java

@@ -38,7 +38,7 @@
 import org.stagemonitor.configuration.source.SystemPropertyConfigurationSource;
 
 import javax.annotation.Nullable;
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -123,11 +123,11 @@ private ConfigurationRegistry getDefaultConfigurationRegistry() {
         }
     }
 
-    private List<? extends ConfigurationSource> getConfigSources() {
-        return Arrays.asList(
-            new PrefixingConfigurationSourceWrapper(new SystemPropertyConfigurationSource(), "elastic.apm."),
-            new PrefixingConfigurationSourceWrapper(new EnvironmentVariableConfigurationSource(), "ELASTIC_APM_"),
-            new AbstractConfigurationSource() {
+    private List<ConfigurationSource> getConfigSources() {
+        List<ConfigurationSource> result = new ArrayList<>();
+        result.add(new PrefixingConfigurationSourceWrapper(new SystemPropertyConfigurationSource(), "elastic.apm."));
+        result.add(new PrefixingConfigurationSourceWrapper(new EnvironmentVariableConfigurationSource(), "ELASTIC_APM_"));
+        result.add(new AbstractConfigurationSource() {
                 @Override
                 public String getValue(String key) {
                     return inlineConfig.get(key);
@@ -137,8 +137,11 @@ public String getValue(String key) {
                 public String getName() {
                     return "Inline configuration";
                 }
-            },
-            new PropertyFileConfigurationSource("elasticapm.properties"));
+            });
+        if (PropertyFileConfigurationSource.isPresent("elasticapm.properties")) {
+            result.add(new PropertyFileConfigurationSource("elasticapm.properties"));
+        }
+        return result;
     }
 
 }