diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 14099ae65b..20d950ad14 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -32,7 +32,11 @@ updates: - dependency-name: "com.datastax.cassandra:cassandra-driver-core" - dependency-name: "com.datastax.oss:java-driver-core" - dependency-name: "io.micrometer:*" + - dependency-name: "jakarta.*:*" ignore: - dependency-name: "net.bytebuddy:byte-buddy-agent" # We deliberately want to keep this older version of Byte Buddy for our runtime attach test versions: ["<=1.9.16"] + - dependency-name: "org.slf4j:slf4j-api" + # A static arbitrary version used within our external plugin test + versions: ["<=1.7.25"] diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index fd8a5eaed0..80b5e6937d 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -34,12 +34,13 @@ endif::[] ===== Bug fixes * Fix runtime attach with some docker images - {pull}2385[#2385] * Restore dynamic capability to `log_level` config for plugin loggers - {pull}2384[#2384] -* Fix slf4j-related `LinkageError` - {pull}2390[#2390] +* Fix slf4j-related `LinkageError` - {pull}2390[#2390] and {pull}2376[#2376] * Fix possible deadlock occurring when Byte Buddy reads System properties by warming up bytecode instrumentation code paths. The BCI warmup is on by default and may be disabled through the internal `warmup_byte_buddy` config option - {pull}2368[#2368] * Fixed few dubbo plugin issues - {pull}2149[#2149] ** Dubbo transaction will should be created at the provider side ** APM headers conversion issue within dubbo transaction +* Fix External plugins automatic setting of span outcome - {pull}2376[#2376] [[release-notes-1.x]] === Java Agent version 1.x diff --git a/apm-agent-common/src/main/java/co/elastic/apm/agent/common/util/AgentInfo.java b/apm-agent-common/src/main/java/co/elastic/apm/agent/common/util/AgentInfo.java new file mode 100644 index 0000000000..b677ceb7c4 --- /dev/null +++ b/apm-agent-common/src/main/java/co/elastic/apm/agent/common/util/AgentInfo.java @@ -0,0 +1,67 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.agent.common.util; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +public class AgentInfo { + + private static final Set dependencyPackages = new HashSet<>(Arrays.asList( + "org.slf4j", + "org.apache.logging", + "net.bytebuddy", + "org.objectweb.asm", + "org.jctools" , + "org.stagemonitor", + "org.HdrHistogram", + "co.elastic.logging", + "com.blogspot.mydailyjava.weaklockfree", + "com.lmax.disruptor", + "com.dslplatform.json", + "com.googlecode.concurrentlinkedhashmap" + )); + + private static final Set agentRootPackages = new HashSet<>(Arrays.asList( + "co.elastic.apm", + "bootstrap.co.elastic.apm", + "bootstrap.java.lang" + )); + + /** + * Returns a list of packages of dependencies used by the agent. + * Should be updated manually whenever a new dependency is added to the agent code. + * See {@code co.elastic.apm.agent.premain.AgentPackagingIT#validateDependencyPackages()}. + * @return a list of root packages of dependencies used by the agent. + */ + public static Set getAgentDependencyPackages() { + return dependencyPackages; + } + + /** + * Returns a list of root packages of agent classes. + * Should be updated manually whenever a new root is added to the agent code. + * See {@code co.elastic.apm.agent.premain.AgentPackagingIT#validateDependencyPackages()}. + * @return a list of agent root packages. + */ + public static Set getAgentRootPackages() { + return agentRootPackages; + } +} diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java index 8bc738928e..7cd988b920 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java @@ -198,12 +198,12 @@ public boolean accept(File dir, String name) { } List result = new ArrayList<>(pluginJars.length); for (File pluginJar : pluginJars) { + logger.info("Loading plugin {}", pluginJar.getName()); try { result.add(new ExternalPluginClassLoader(pluginJar, ElasticApmAgent.class.getClassLoader())); - } catch (IOException e) { - logger.error(e.getMessage(), e); + } catch (Exception e) { + logger.error("Error loading external plugin", e); } - logger.info("Loading plugin {}", pluginJar.getName()); } return result; } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java index f31ec8ff94..a1dbe47967 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/IndyBootstrap.java @@ -207,6 +207,7 @@ public class IndyBootstrap { * Caches the names of classes that are defined within a package and it's subpackages */ private static final ConcurrentMap> classesByPackage = new ConcurrentHashMap<>(); + @Nullable static Method indyBootstrapMethod; @@ -362,10 +363,19 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup, MethodHandle instrumentedMethod = args.length >= 5 ? (MethodHandle) args[4] : null; ClassLoader instrumentationClassLoader = ElasticApmAgent.getInstrumentationClassLoader(adviceClassName); + ClassLoader targetClassLoader = lookup.lookupClass().getClassLoader(); ClassFileLocator classFileLocator; List pluginClasses = new ArrayList<>(); if (instrumentationClassLoader instanceof ExternalPluginClassLoader) { - pluginClasses.addAll(((ExternalPluginClassLoader) instrumentationClassLoader).getClassNames()); + List externalPluginClasses = ((ExternalPluginClassLoader) instrumentationClassLoader).getClassNames(); + for (String externalPluginClass : externalPluginClasses) { + if (// API classes have no dependencies and don't need to be loaded by an IndyPluginCL + !(externalPluginClass.startsWith("co.elastic.apm.api")) && + !(externalPluginClass.startsWith("co.elastic.apm.opentracing")) + ) { + pluginClasses.add(externalPluginClass); + } + } File agentJarFile = ElasticApmAgent.getAgentJarFile(); if (agentJarFile == null) { throw new IllegalStateException("External plugin cannot be applied - can't find agent jar"); @@ -380,7 +390,7 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup, } pluginClasses.add(LOOKUP_EXPOSER_CLASS_NAME); ClassLoader pluginClassLoader = IndyPluginClassLoaderFactory.getOrCreatePluginClassLoader( - lookup.lookupClass().getClassLoader(), + targetClassLoader, pluginClasses, // we provide the instrumentation class loader as the agent class loader, but it could actually be an // ExternalPluginClassLoader, of which parent is the agent class loader, so this works as well. diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java index f0a63bccb9..48c87943db 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java @@ -22,6 +22,7 @@ import co.elastic.apm.agent.matcher.WildcardMatcher; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; +import co.elastic.apm.agent.util.ClassLoaderUtils; import co.elastic.apm.agent.util.Version; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.annotation.AnnotationSource; @@ -54,7 +55,7 @@ public class CustomElementMatchers { private static final ElementMatcher.Junction.AbstractBase AGENT_CLASS_LOADER_MATCHER = new ElementMatcher.Junction.AbstractBase() { @Override public boolean matches(@Nullable ClassLoader classLoader) { - return classLoader != null && classLoader.getClass().getName().startsWith("co.elastic.apm"); + return ClassLoaderUtils.isAgentClassLoader(classLoader); } }; diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/ExternalPluginClassLoader.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/ExternalPluginClassLoader.java index 76aff8e4dc..232339a7a8 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/ExternalPluginClassLoader.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/ExternalPluginClassLoader.java @@ -18,8 +18,10 @@ */ package co.elastic.apm.agent.bci.classloading; +import co.elastic.apm.agent.common.util.AgentInfo; import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.sdk.ElasticApmInstrumentation; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; import java.io.File; import java.io.IOException; @@ -44,7 +46,8 @@ public ExternalPluginClassLoader(File pluginJar, ClassLoader agentClassLoader) t super(new URL[]{pluginJar.toURI().toURL()}, agentClassLoader); classNames = Collections.unmodifiableList(scanForClasses(pluginJar)); if (classNames.contains(ElasticApmInstrumentation.class.getName())) { - throw new IllegalStateException("The plugin %s contains the plugin SDK. Please make sure the scope for the dependency apm-agent-plugin-sdk is set to provided."); + throw new IllegalStateException(String.format("The plugin %s contains the plugin SDK. Please make sure the " + + "scope for the dependency apm-agent-plugin-sdk is set to provided.", pluginJar.getName())); } } @@ -55,7 +58,27 @@ private List scanForClasses(File pluginJar) throws IOException { while (entries.hasMoreElements()) { JarEntry jarEntry = entries.nextElement(); if (jarEntry.getName().endsWith(".class")) { - tempClassNames.add(jarEntry.getName().replace('/', '.').substring(0, jarEntry.getName().length() - 6)); + String fqcn = jarEntry.getName().replace('/', '.').substring(0, jarEntry.getName().length() - 6); + if (fqcn.startsWith("org.slf4j") || fqcn.startsWith("org.apache.logging")) { + throw new IllegalStateException(String.format("Package \"%s\" is used within plugin %s. This is not allowed " + + "because it is already used by the agent. For logging purposes, use the agent SDK logging facade - %s", + fqcn.substring(0, fqcn.lastIndexOf('.')), + pluginJar.getName(), + LoggerFactory.class.getName() + )); + } + for (String agentDependencyPackage : AgentInfo.getAgentDependencyPackages()) { + if (fqcn.startsWith(agentDependencyPackage)) { + throw new IllegalStateException(String.format("Package \"%s\" is used within plugin %s. This is not allowed " + + "because the same dependency is used by the agent. Please either replace the corresponding dependency or " + + "make sure its scope is set to provided. See the full list of such packages in %s", + fqcn.substring(0, fqcn.lastIndexOf('.')), + pluginJar.getName(), + AgentInfo.class.getName() + )); + } + } + tempClassNames.add(fqcn); } } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java index 9d066b3913..244c0112b7 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java @@ -24,7 +24,6 @@ import co.elastic.apm.agent.context.ClosableLifecycleListenerAdapter; import co.elastic.apm.agent.context.LifecycleListener; import co.elastic.apm.agent.impl.error.ErrorCapture; -import co.elastic.apm.agent.impl.metadata.MetaData; import co.elastic.apm.agent.impl.metadata.MetaDataFuture; import co.elastic.apm.agent.impl.sampling.ProbabilitySampler; import co.elastic.apm.agent.impl.sampling.Sampler; @@ -57,11 +56,9 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Deque; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java index e83b9c44e5..8c436c0929 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java @@ -390,15 +390,18 @@ public boolean isExit() { return isExit; } - - public void captureException(long epochMicros, Throwable t) { - tracer.captureAndReportException(epochMicros, t, this); + @Nullable + public String captureExceptionAndGetErrorId(long epochMicros, @Nullable Throwable t) { + if (t != null) { + hasCapturedExceptions = true; + return tracer.captureAndReportException(epochMicros, t, this); + } + return null; } public T captureException(@Nullable Throwable t) { if (t != null) { - hasCapturedExceptions = true; - captureException(getTraceContext().getClock().getEpochMicros(), t); + captureExceptionAndGetErrorId(getTraceContext().getClock().getEpochMicros(), t); } return (T) this; } @@ -409,7 +412,7 @@ public void endExceptionally(@Nullable Throwable t) { @Nullable public String captureExceptionAndGetErrorId(@Nullable Throwable t) { - return tracer.captureAndReportException(getTraceContext().getClock().getEpochMicros(), t, this); + return captureExceptionAndGetErrorId(getTraceContext().getClock().getEpochMicros(), t); } public void addLabel(String key, String value) { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index 3e8b410148..cde3f6126a 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -26,6 +26,7 @@ import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; import co.elastic.apm.agent.util.ByteUtils; +import co.elastic.apm.agent.util.ClassLoaderUtils; import co.elastic.apm.agent.util.HexUtils; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; @@ -712,14 +713,15 @@ public int hashCode() { } void setApplicationClassLoader(@Nullable ClassLoader classLoader) { - if (classLoader != null) { - WeakReference local = classLoaderWeakReferenceCache.get(classLoader); - if (local == null) { - local = new WeakReference<>(classLoader); - classLoaderWeakReferenceCache.putIfAbsent(classLoader, local); - } - applicationClassLoader = local; + if (ClassLoaderUtils.isBootstrapClassLoader(classLoader) || ClassLoaderUtils.isAgentClassLoader(classLoader)) { + return; + } + WeakReference local = classLoaderWeakReferenceCache.get(classLoader); + if (local == null) { + local = new WeakReference<>(classLoader); + classLoaderWeakReferenceCache.putIfAbsent(classLoader, local); } + applicationClassLoader = local; } @Nullable diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/util/ClassLoaderUtils.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/ClassLoaderUtils.java index 1d78920eea..e79e820c3b 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/util/ClassLoaderUtils.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/ClassLoaderUtils.java @@ -42,4 +42,12 @@ public static boolean isPersistentClassLoader(@Nullable ClassLoader classLoader) return false; } } + + public static boolean isAgentClassLoader(@Nullable ClassLoader classLoader) { + return classLoader != null && classLoader.getClass().getName().startsWith("co.elastic.apm"); + } + + public static boolean isBootstrapClassLoader(@Nullable ClassLoader classLoader) { + return classLoader == null; + } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java index e9887d2c7e..738949f2b8 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java @@ -27,6 +27,7 @@ import co.elastic.apm.agent.impl.sampling.ConstantSampler; import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration; import co.elastic.apm.agent.impl.transaction.AbstractSpan; +import co.elastic.apm.agent.impl.transaction.Outcome; import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.agent.impl.transaction.Transaction; @@ -484,6 +485,7 @@ void testCaptureExceptionAndGetErrorId() { Transaction transaction = startTestRootTransaction(); String errorId = transaction.captureExceptionAndGetErrorId(new Exception("test")); transaction.end(); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.FAILURE); assertThat(reporter.getErrors()).hasSize(1); assertThat(errorId).isNotNull(); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java index 92e584d083..cd84e97cba 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java @@ -255,6 +255,8 @@ public void testGetErrorIdWithTransactionCaptureException() { } endTransaction(); assertThat(errorId).isNotNull(); + assertThat(reporter.getTransactions()).hasSize(1); + assertThat(reporter.getFirstTransaction().getOutcome()).isEqualTo(convertOutcome(Outcome.FAILURE)); } @Test @@ -271,6 +273,10 @@ public void testGetErrorIdWithSpanCaptureException() { } endTransaction(); assertThat(errorId).isNotNull(); + assertThat(reporter.getTransactions()).hasSize(1); + assertThat(reporter.getFirstTransaction().getOutcome()).isEqualTo(convertOutcome(Outcome.SUCCESS)); + assertThat(reporter.getSpans()).hasSize(1); + assertThat(reporter.getFirstSpan().getOutcome()).isEqualTo(convertOutcome(Outcome.FAILURE)); } @Test @@ -308,4 +314,8 @@ private void endTransaction() { transaction.end(); assertThat(reporter.getTransactions()).hasSize(1); } + + private co.elastic.apm.agent.impl.transaction.Outcome convertOutcome(Outcome apiOutcome) { + return co.elastic.apm.agent.impl.transaction.Outcome.valueOf(apiOutcome.toString()); + } } diff --git a/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java b/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java index c6c79fed14..154ad78f71 100644 --- a/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java +++ b/apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java @@ -125,7 +125,7 @@ public static void log(@Advice.FieldValue(value = "dispatcher", typing = Assigne final Object errorObject = fields.get("error.object"); if (errorObject instanceof Throwable) { if (epochTimestampMicros > 0) { - span.captureException(epochTimestampMicros, (Throwable) errorObject); + span.captureExceptionAndGetErrorId(epochTimestampMicros, (Throwable) errorObject); } else { span.captureException((Throwable) errorObject); } diff --git a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java index 646ccc44cb..d8e363adeb 100644 --- a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java +++ b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java @@ -24,6 +24,7 @@ import co.elastic.apm.agent.impl.context.Http; import co.elastic.apm.agent.impl.context.web.ResultUtil; import co.elastic.apm.agent.impl.transaction.Id; +import co.elastic.apm.agent.impl.transaction.Outcome; import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.agent.impl.transaction.Transaction; import io.opentracing.Scope; @@ -81,9 +82,11 @@ void testCreateNonActiveTransaction() { span.finish(TimeUnit.MILLISECONDS.toMicros(1)); assertThat(reporter.getTransactions()).hasSize(1); - assertThat(reporter.getFirstTransaction().getDuration()).isEqualTo(1000); - assertThat(reporter.getFirstTransaction().getNameAsString()).isEqualTo("test"); - assertThat(reporter.getFirstTransaction().getFrameworkName()).isEqualTo("OpenTracing"); + Transaction transaction = reporter.getFirstTransaction(); + assertThat(transaction.getDuration()).isEqualTo(1000); + assertThat(transaction.getNameAsString()).isEqualTo("test"); + assertThat(transaction.getFrameworkName()).isEqualTo("OpenTracing"); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.SUCCESS); } @Test @@ -140,6 +143,7 @@ void testContextAvailableAfterFinish() { assertThat(reporter.getTransactions()).hasSize(1); final Transaction transaction = reporter.getFirstTransaction(); String transactionId = transaction.getTraceContext().getId().toString(); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.SUCCESS); final Span childSpan = apmTracer.buildSpan("span") .asChildOf(span.context()) @@ -147,7 +151,9 @@ void testContextAvailableAfterFinish() { childSpan.finish(); assertThat(reporter.getSpans()).hasSize(1); - assertThat(reporter.getFirstSpan().getTraceContext().getParentId().toString()).isEqualTo(transactionId); + co.elastic.apm.agent.impl.transaction.Span internalSpan = reporter.getFirstSpan(); + assertThat(internalSpan.getTraceContext().getParentId().toString()).isEqualTo(transactionId); + assertThat(internalSpan.getOutcome()).isEqualTo(Outcome.SUCCESS); } @Test @@ -157,6 +163,7 @@ void testScopeAfterFinish() { assertThat(reporter.getTransactions()).hasSize(1); final Transaction transaction = reporter.getFirstTransaction(); String transactionId = transaction.getTraceContext().getId().toString(); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.SUCCESS); try (Scope scope = apmTracer.activateSpan(span)) { final Span childSpan = apmTracer.buildSpan("span") @@ -165,7 +172,9 @@ void testScopeAfterFinish() { } assertThat(reporter.getSpans()).hasSize(1); - assertThat(reporter.getFirstSpan().getTraceContext().getParentId().toString()).isEqualTo(transactionId); + co.elastic.apm.agent.impl.transaction.Span internalSpan = reporter.getFirstSpan(); + assertThat(internalSpan.getTraceContext().getParentId().toString()).isEqualTo(transactionId); + assertThat(internalSpan.getOutcome()).isEqualTo(Outcome.SUCCESS); } @Test @@ -401,11 +410,34 @@ void testErrorLogging() { span.finish(); } assertThat(reporter.getTransactions()).hasSize(1); - assertThat(reporter.getFirstTransaction().getResult()).isEqualTo("error"); + Transaction transaction = reporter.getFirstTransaction(); + assertThat(transaction.getResult()).isEqualTo("error"); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.FAILURE); assertThat(reporter.getErrors()).hasSize(1); assertThat(reporter.getFirstError().getException()).isNotNull(); assertThat(reporter.getFirstError().getException().getMessage()).isEqualTo("Catch me if you can"); - assertThat(reporter.getFirstError().getTraceContext().getParentId()).isEqualTo(reporter.getFirstTransaction().getTraceContext().getId()); + assertThat(reporter.getFirstError().getTraceContext().getParentId()).isEqualTo(transaction.getTraceContext().getId()); + } + + @Test + void testErrorLoggingWithTimestamp() { + Span span = apmTracer.buildSpan("someWork").start(); + try (Scope scope = apmTracer.activateSpan(span)) { + throw new RuntimeException("Catch me if you can"); + } catch (Exception ex) { + Tags.ERROR.set(span, true); + span.log(System.currentTimeMillis(), Map.of(Fields.EVENT, "error", Fields.ERROR_OBJECT, ex, Fields.MESSAGE, ex.getMessage())); + } finally { + span.finish(); + } + assertThat(reporter.getTransactions()).hasSize(1); + Transaction transaction = reporter.getFirstTransaction(); + assertThat(transaction.getResult()).isEqualTo("error"); + assertThat(transaction.getOutcome()).isEqualTo(Outcome.FAILURE); + assertThat(reporter.getErrors()).hasSize(1); + assertThat(reporter.getFirstError().getException()).isNotNull(); + assertThat(reporter.getFirstError().getException().getMessage()).isEqualTo("Catch me if you can"); + assertThat(reporter.getFirstError().getTraceContext().getParentId()).isEqualTo(transaction.getTraceContext().getId()); } @Test @@ -420,6 +452,7 @@ void testErrorLoggingWithoutScope() { span.finish(); } assertThat(reporter.getTransactions()).hasSize(1); + assertThat(reporter.getFirstTransaction().getOutcome()).isEqualTo(Outcome.FAILURE); assertThat(reporter.getErrors()).hasSize(1); assertThat(reporter.getFirstError().getException()).isNotNull(); assertThat(reporter.getFirstError().getException().getMessage()).isEqualTo("Catch me if you can"); diff --git a/elastic-apm-agent/src/test/java/co/elastic/apm/agent/premain/AgentPackagingIT.java b/elastic-apm-agent/src/test/java/co/elastic/apm/agent/premain/AgentPackagingIT.java index 922b5a3bb2..cc213e2f65 100644 --- a/elastic-apm-agent/src/test/java/co/elastic/apm/agent/premain/AgentPackagingIT.java +++ b/elastic-apm-agent/src/test/java/co/elastic/apm/agent/premain/AgentPackagingIT.java @@ -18,6 +18,8 @@ */ package co.elastic.apm.agent.premain; +import co.elastic.apm.agent.common.util.AgentInfo; +import org.assertj.core.description.Description; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -31,9 +33,11 @@ import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import static org.assertj.core.api.Assertions.assertThat; @@ -123,11 +127,37 @@ void duplicatedClassesCheck() throws IOException { classLocation.put(path, location); } + }); + } + @Test + void validateDependencyPackages() throws IOException { - }); + Set agentDependencyPackages = AgentInfo.getAgentDependencyPackages(); + Set packagesAsPaths = agentDependencyPackages.stream() + .map(packageName -> packageName.replace('.', '/')) + .collect(Collectors.toSet()); + packagesAsPaths.addAll(AgentInfo.getAgentRootPackages().stream() + .map(packageName -> packageName.replace('.', '/')) + .collect(Collectors.toSet())); + JarFile jarFile = new JarFile(agentJar.toFile()); + String shadedClassesDir = "agent/"; + jarFile.stream() + .map(ZipEntry::getName) + .filter(name -> name.startsWith(shadedClassesDir)) + .filter(name -> name.endsWith(".esclazz")) + .map(name -> name.substring(shadedClassesDir.length())) + .filter(name -> name.lastIndexOf('/') > 0) + .forEach(name -> assertThat(packagesAsPaths.stream().anyMatch(name::startsWith)) + .describedAs(new Description() { + @Override + public String value() { + String packageName = name.substring(0, name.lastIndexOf('/')).replace('/', '.'); + return String.format("Package %s is used by the agent and not declared by co.elastic.apm.agent.premain.Utils.getAgentDependencyPackages", packageName); + } + }) + .isTrue()); } - } diff --git a/integration-tests/application-server-integration-tests/pom.xml b/integration-tests/application-server-integration-tests/pom.xml index 9b01bf7ff5..2d34881003 100644 --- a/integration-tests/application-server-integration-tests/pom.xml +++ b/integration-tests/application-server-integration-tests/pom.xml @@ -77,6 +77,12 @@ ${project.version} war + + ${project.groupId} + external-plugin-jakarta-app + ${project.version} + war + ${project.groupId} diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java index c197e13192..d278d2f60a 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java @@ -102,7 +102,7 @@ public abstract class AbstractServletContainerIntegrationTest { private static final String AGENT_VERSION_TO_DOWNLOAD_FROM_MAVEN = null; private static MockServerContainer mockServerContainer = new MockServerContainer() - .withLogConsumer(TestContainersUtils.createSlf4jLogConsumer(MockServerContainer.class)) + //.withLogConsumer(TestContainersUtils.createSlf4jLogConsumer(MockServerContainer.class)) .withNetworkAliases("apm-server") .withNetwork(Network.SHARED); private static OkHttpClient httpClient; @@ -156,6 +156,12 @@ protected AbstractServletContainerIntegrationTest(GenericContainer servletCon List ignoreUrls = new ArrayList<>(); for (TestApp app : getTestApps()) { ignoreUrls.add(String.format("/%s/status*", app.getDeploymentContext())); + for (String ignorePath : app.getPathsToIgnore()) { + if (ignorePath.startsWith("/")) { + ignorePath = ignorePath.substring(1); + } + ignoreUrls.add(String.format("/%s/%s", app.getDeploymentContext(), ignorePath)); + } } ignoreUrls.add("/favicon.ico"); String ignoreUrlConfig = String.join(",", ignoreUrls); @@ -598,7 +604,7 @@ private void validateServiceName(JsonNode event) { .withFailMessage("No service name set. Expected '%s'. Event was %s", expectedServiceName, event) .isNotNull(); assertThat(contextService.get("name").textValue()) - .describedAs("Event has non-expected service name %s", event) + .describedAs("Event has unexpected service name %s", event) .isEqualTo(expectedServiceName); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractTomcatIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractTomcatIT.java index 8ba5c4b3f6..57beb5cdeb 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractTomcatIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractTomcatIT.java @@ -33,7 +33,7 @@ public AbstractTomcatIT(final String tomcatVersion) { @Override protected void enableDebugging(GenericContainer servletContainer) { - servletContainer.withEnv("CATALINA_OPTS", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005"); + servletContainer.withEnv("CATALINA_OPTS", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005"); } @Nullable diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JBossIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JBossIT.java index 3b28d61c33..24e9c26de9 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JBossIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JBossIT.java @@ -19,8 +19,8 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; import co.elastic.apm.servlet.tests.JBossServletApiTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp; import co.elastic.apm.servlet.tests.SoapTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -68,7 +68,7 @@ protected Iterable> getTestClasses() { JsfApplicationServerTestApp.class, SoapTestApp.class, CdiApplicationServerTestApp.class, - ExternalPluginTestApp.class + JavaxExternalPluginTestApp.class ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeJettyIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeJettyIT.java index dd3e1c93dd..51e794c74f 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeJettyIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeJettyIT.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.servlet; +import co.elastic.apm.servlet.tests.JakartaExternalPluginTestApp; import co.elastic.apm.servlet.tests.JakartaeeJsfServletContainerTestApp; import co.elastic.apm.servlet.tests.JakartaeeServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -40,6 +41,10 @@ public static Iterable data() { @Override protected Iterable> getTestClasses() { - return Arrays.asList(JakartaeeServletApiTestApp.class, JakartaeeJsfServletContainerTestApp.class); + return Arrays.asList( + JakartaeeServletApiTestApp.class, + JakartaeeJsfServletContainerTestApp.class, + JakartaExternalPluginTestApp.class + ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeTomcatIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeTomcatIT.java index a17c946574..53884728f2 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeTomcatIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JakartaeeTomcatIT.java @@ -19,6 +19,7 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiJakartaeeServletContainerTestApp; +import co.elastic.apm.servlet.tests.JakartaExternalPluginTestApp; import co.elastic.apm.servlet.tests.JakartaeeJsfServletContainerTestApp; import co.elastic.apm.servlet.tests.JakartaeeServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -40,8 +41,7 @@ public static Iterable data() { {"10.0.10-jdk8"}, {"10.0.10-jdk11"}, {"10.0.10-jdk8-adoptopenjdk-openj9"}, - // TODO openj9 on JDK11 has an access problem from java.base - //{"10.0.10-jdk11-adoptopenjdk-openj9"}, + {"10.0.10-jdk11-adoptopenjdk-openj9"}, {"10.0.10-jdk8-adoptopenjdk-hotspot"}, {"10.0.10-jdk11-adoptopenjdk-hotspot"}, }); @@ -51,6 +51,7 @@ public static Iterable data() { protected Iterable> getTestClasses() { return Arrays.asList(JakartaeeServletApiTestApp.class, JakartaeeJsfServletContainerTestApp.class, + JakartaExternalPluginTestApp.class, CdiJakartaeeServletContainerTestApp.class); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java index a556876c61..c1ac87fbea 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java @@ -18,7 +18,7 @@ */ package co.elastic.apm.servlet; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfServletContainerTestApp; import co.elastic.apm.servlet.tests.ServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -26,7 +26,6 @@ import org.junit.runners.Parameterized; import java.util.Arrays; -import java.util.List; @RunWith(Parameterized.class) public class JettyIT extends AbstractJettyIT { @@ -42,6 +41,10 @@ public static Iterable data() { @Override protected Iterable> getTestClasses() { - return Arrays.asList(ServletApiTestApp.class, JsfServletContainerTestApp.class, ExternalPluginTestApp.class); + return Arrays.asList( + ServletApiTestApp.class, + JsfServletContainerTestApp.class, + JavaxExternalPluginTestApp.class + ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/PayaraIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/PayaraIT.java index 069759acab..987e860dfe 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/PayaraIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/PayaraIT.java @@ -19,11 +19,10 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp; import co.elastic.apm.servlet.tests.ServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; -import org.junit.Ignore; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.testcontainers.containers.GenericContainer; @@ -76,7 +75,7 @@ protected Iterable> getTestClasses() { ServletApiTestApp.class, JsfApplicationServerTestApp.class, CdiApplicationServerTestApp.class, - ExternalPluginTestApp.class + JavaxExternalPluginTestApp.class ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java index 7ff8a75947..b09f58aa83 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java @@ -19,7 +19,7 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiServletContainerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfServletContainerTestApp; import co.elastic.apm.servlet.tests.ServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -58,7 +58,7 @@ protected Iterable> getTestClasses() { List> testClasses = new ArrayList<>(); testClasses.add(ServletApiTestApp.class); testClasses.add(CdiServletContainerTestApp.class); - testClasses.add(ExternalPluginTestApp.class); + testClasses.add(JavaxExternalPluginTestApp.class); if (!getImageName().contains("jre7")) { // The JSF test app depends on myfaces 2.3.2 which requires Java 8 or higher testClasses.add(JsfServletContainerTestApp.class); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebLogicIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebLogicIT.java index dfbc991682..d50ec99c9f 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebLogicIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebLogicIT.java @@ -19,7 +19,7 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp; import co.elastic.apm.servlet.tests.ServletApiTestApp; import co.elastic.apm.servlet.tests.SoapTestApp; @@ -66,7 +66,7 @@ protected Iterable> getTestClasses() { JsfApplicationServerTestApp.class, SoapTestApp.class, CdiApplicationServerTestApp.class, - ExternalPluginTestApp.class + JavaxExternalPluginTestApp.class ); } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java index d68b6f94fe..2f81d9747f 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java @@ -19,7 +19,7 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp; import co.elastic.apm.servlet.tests.ServletApiTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -74,7 +74,7 @@ protected Iterable> getTestClasses() { ServletApiTestApp.class, JsfApplicationServerTestApp.class, CdiApplicationServerTestApp.class, - ExternalPluginTestApp.class + JavaxExternalPluginTestApp.class ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WildFlyIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WildFlyIT.java index de4b480ae1..32f44eb2fa 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WildFlyIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WildFlyIT.java @@ -19,8 +19,8 @@ package co.elastic.apm.servlet; import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp; -import co.elastic.apm.servlet.tests.ExternalPluginTestApp; import co.elastic.apm.servlet.tests.JBossServletApiTestApp; +import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp; import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp; import co.elastic.apm.servlet.tests.SoapTestApp; import co.elastic.apm.servlet.tests.TestApp; @@ -72,7 +72,7 @@ protected Iterable> getTestClasses() { JsfApplicationServerTestApp.class, SoapTestApp.class, CdiApplicationServerTestApp.class, - ExternalPluginTestApp.class + JavaxExternalPluginTestApp.class ); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ExternalPluginTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ExternalPluginTestApp.java index 59d62ff0b6..e35a40b441 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ExternalPluginTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ExternalPluginTestApp.java @@ -18,25 +18,36 @@ */ package co.elastic.apm.servlet.tests; +import co.elastic.apm.agent.impl.Tracer; +import co.elastic.apm.agent.impl.transaction.Outcome; import co.elastic.apm.servlet.AbstractServletContainerIntegrationTest; import com.fasterxml.jackson.databind.JsonNode; import okhttp3.Response; +import javax.annotation.Nullable; import java.io.File; +import java.io.IOException; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; -public class ExternalPluginTestApp extends TestApp { - public ExternalPluginTestApp() { - super("../external-plugin-test/external-plugin-app", - "external-plugin-webapp.war", - "external-plugin-webapp", +public abstract class ExternalPluginTestApp extends TestApp { + + private final String appWarFileName; + private boolean isServiceNameExpected; + + protected ExternalPluginTestApp(String testAppModuleName, String appWarFileName) { + super("../external-plugin-test/" + testAppModuleName, + appWarFileName + ".war", + appWarFileName, "status.html", - "external-plugin-webapp" + appWarFileName ); + this.appWarFileName = appWarFileName; } @Override @@ -52,19 +63,56 @@ public Map getAdditionalFilesToBind() { @Override public Map getAdditionalEnvVariables() { - return Map.of("ELASTIC_APM_PLUGINS_DIR", "/plugins"); + return Map.of( + "ELASTIC_APM_PLUGINS_DIR", "/plugins", + "ELASTIC_APM_ENABLE_LOG_CORRELATION", "true" + ); + } + + @Override + public Collection getPathsToIgnore() { + return List.of("/test-transaction-external-plugin"); } @Override - public void test(AbstractServletContainerIntegrationTest test) throws Exception { - final Response response = test.executeRequest("/external-plugin-webapp/test-external-plugin", null); + public void test(AbstractServletContainerIntegrationTest containerIntegrationTest) throws Exception { + executeTest(containerIntegrationTest, true, this::testSpanReporting); + // service name is not expected to be captured for the plugin transaction creation test + executeTest(containerIntegrationTest, false, this::testTransactionReporting); + } + + private void executeTest(final AbstractServletContainerIntegrationTest containerIntegrationTest, + boolean isServiceNameExpected, + Test test) throws IOException { + containerIntegrationTest.clearMockServerLog(); + this.isServiceNameExpected = isServiceNameExpected; + test.execute(containerIntegrationTest); + } + + /** + * Since we test custom transaction creation through the external plugin, the service name for this transaction cannot be + * captured through the {@link Tracer#overrideServiceNameForClassLoader(java.lang.ClassLoader, java.lang.String)} mechanism. + */ + @Nullable + @Override + public String getExpectedServiceName() { + if (!isServiceNameExpected) { + return null; + } + return super.getExpectedServiceName(); + } + + void testSpanReporting(AbstractServletContainerIntegrationTest test) throws IOException { + final Response response = test.executeRequest(constructFullPath("test-span-external-plugin"), null); assertThat(response.code()).isEqualTo(200); - assertThat(response.body().string()).isNotEmpty(); + assertThat(Objects.requireNonNull(response.body()).string()).isNotEmpty(); final List reportedTransactions = test.getReportedTransactions(); assertThat(reportedTransactions).hasSize(1); - String traceId = reportedTransactions.get(0).get("trace_id").textValue(); - String transactionId = reportedTransactions.get(0).get("id").textValue(); + JsonNode transaction = reportedTransactions.get(0); + String traceId = transaction.get("trace_id").textValue(); + String transactionId = transaction.get("id").textValue(); + assertThat(transaction.get("outcome").textValue()).isEqualTo(Outcome.SUCCESS.toString()); List reportedSpans = test.getReportedSpans(); assertThat(reportedSpans).hasSize(1); JsonNode span = reportedSpans.get(0); @@ -72,14 +120,44 @@ public void test(AbstractServletContainerIntegrationTest test) throws Exception assertThat(span.get("trace_id").textValue()).isEqualTo(traceId); assertThat(span.get("parent_id").textValue()).isEqualTo(transactionId); assertThat(span.get("type").textValue()).isEqualTo("plugin.external.trace"); + assertThat(span.get("outcome").textValue()).isEqualTo(Outcome.FAILURE.toString()); List reportedErrors = test.getReportedErrors(); assertThat(reportedErrors).hasSize(1); JsonNode error = reportedErrors.get(0); assertThat(error.get("transaction_id").textValue()).isEqualTo(transactionId); - assertThat(error.get("context").get("service").get("name").textValue()).isEqualTo("external-plugin-webapp"); + assertThat(error.get("context").get("service").get("name").textValue()).isEqualTo(appWarFileName); JsonNode exception = error.get("exception"); assertThat(exception.get("message").textValue()).isEqualTo("Test Exception"); assertThat(exception.get("type").textValue()).isEqualTo("java.lang.IllegalStateException"); } + void testTransactionReporting(AbstractServletContainerIntegrationTest test) throws IOException { + final Response response = test.executeRequest(constructFullPath("test-transaction-external-plugin"), null); + assertThat(response.code()).isEqualTo(200); + assertThat(Objects.requireNonNull(response.body()).string()).isNotEmpty(); + + final List reportedTransactions = test.getReportedTransactions(); + assertThat(reportedTransactions).hasSize(1); + JsonNode transaction = reportedTransactions.get(0); + assertThat(transaction.get("name").textValue()).isEqualTo("TestClass#traceMe"); + assertThat(transaction.get("type").textValue()).isEqualTo("custom"); + assertThat(transaction.get("outcome").textValue()).isEqualTo(Outcome.FAILURE.toString()); + List reportedErrors = test.getReportedErrors(); + assertThat(reportedErrors).hasSize(1); + JsonNode error = reportedErrors.get(0); + String transactionId = transaction.get("id").textValue(); + assertThat(error.get("transaction_id").textValue()).isEqualTo(transactionId); + JsonNode exception = error.get("exception"); + assertThat(exception.get("message").textValue()).isEqualTo("Test Exception"); + assertThat(exception.get("type").textValue()).isEqualTo("java.lang.IllegalStateException"); + } + + private String constructFullPath(String path) { + return String.format("/%s/%s", appWarFileName, path); + } + + @FunctionalInterface + private interface Test { + void execute(AbstractServletContainerIntegrationTest containerIntegrationTest) throws IOException; + } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaExternalPluginTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaExternalPluginTestApp.java new file mode 100644 index 0000000000..d3fec82cfc --- /dev/null +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaExternalPluginTestApp.java @@ -0,0 +1,26 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.servlet.tests; + +public class JakartaExternalPluginTestApp extends ExternalPluginTestApp { + + public JakartaExternalPluginTestApp() { + super("external-plugin-jakarta-app", "external-plugin-jakarta-webapp"); + } +} diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JavaxExternalPluginTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JavaxExternalPluginTestApp.java new file mode 100644 index 0000000000..9606a8af28 --- /dev/null +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JavaxExternalPluginTestApp.java @@ -0,0 +1,26 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.servlet.tests; + +public class JavaxExternalPluginTestApp extends ExternalPluginTestApp { + + public JavaxExternalPluginTestApp() { + super("external-plugin-app", "external-plugin-webapp"); + } +} diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/TestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/TestApp.java index d21358df49..7b96068b0d 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/TestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/TestApp.java @@ -21,6 +21,7 @@ import co.elastic.apm.servlet.AbstractServletContainerIntegrationTest; import javax.annotation.Nullable; +import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -71,6 +72,15 @@ public Map getAdditionalFilesToBind() { return Collections.emptyMap(); } + /** + * Provides a way for test apps to configure ignored URLs + * + * @return a collection of URL paths that will be appended to the {@link #getDeploymentContext() app context} + */ + public Collection getPathsToIgnore() { + return Collections.emptyList(); + } + /** * Provides a way to configure additional environment variables for a specific app * diff --git a/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestServlet.java b/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java similarity index 88% rename from integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestServlet.java rename to integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java index e4ae991dc0..15633e59dc 100644 --- a/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestServlet.java +++ b/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java @@ -27,8 +27,11 @@ * `integration-tests/application-server-integration-tests` (see {@code ExternalPluginTestApp}). When tested this way, * the plugin is loaded as a jar from the configured plugin directory, including the instrumentation class itself and * the META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation file. + * + * Since a transaction is created through the Servlet instrumentation, the instrumented method ({@link TestClass#traceMe(boolean)}) + * invocation targeted by the external plugin should be captured as a span. */ -public class ExternalPluginTestServlet extends javax.servlet.http.HttpServlet { +public class ExternalPluginTestSpanServlet extends javax.servlet.http.HttpServlet { protected void doGet(javax.servlet.http.HttpServletRequest request, javax.servlet.http.HttpServletResponse response) throws javax.servlet.ServletException, IOException { try { diff --git a/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java b/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java new file mode 100644 index 0000000000..73ff7f32a4 --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java @@ -0,0 +1,50 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.external.plugin.servlet; + +import co.elastic.apm.plugin.test.TestClass; + +import java.io.IOException; + +/** + * This Servlet demonstrates a full integration test through a webapp that is tested on all Servlet containers in + * `integration-tests/application-server-integration-tests` (see {@code ExternalPluginTestApp}). When tested this way, + * the plugin is loaded as a jar from the configured plugin directory, including the instrumentation class itself and + * the META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation file. + * + * Since the path corresponding this Servlet is ignored, a transaction is created through the Servlet instrumentation. + * Therefore, the instrumented method ({@link TestClass#traceMe(boolean)}) invocation targeted by the external plugin + * should be captured as a transaction. + */ +public class ExternalPluginTestTransactionServlet extends javax.servlet.http.HttpServlet { + + protected void doGet(javax.servlet.http.HttpServletRequest request, javax.servlet.http.HttpServletResponse response) throws javax.servlet.ServletException, IOException { + try { + // Using an external library type that is expected to be instrumented by the external plugin. + // A transaction should not be created by the general Servlet API instrumentation because we ignore the + // corresponding URL, so we expect to get a transaction for the TestClass#traceMe() invocation. + // We also expect an error related to this transaction. + // See ExternalPluginTestApp in `integration-tests/application-server-integration-tests`. + new TestClass().traceMe(true); + } catch (IllegalStateException e) { + // do nothing - expected + } + response.getWriter().append("Success"); + } +} diff --git a/integration-tests/external-plugin-test/external-plugin-app/src/main/webapp/WEB-INF/web.xml b/integration-tests/external-plugin-test/external-plugin-app/src/main/webapp/WEB-INF/web.xml index 9923a1026b..a0909f0f7a 100644 --- a/integration-tests/external-plugin-test/external-plugin-app/src/main/webapp/WEB-INF/web.xml +++ b/integration-tests/external-plugin-test/external-plugin-app/src/main/webapp/WEB-INF/web.xml @@ -6,11 +6,20 @@ version="3.0"> - ExternalPluginTestServlet - co.elastic.apm.external.plugin.servlet.ExternalPluginTestServlet + ExternalPluginTestSpanServlet + co.elastic.apm.external.plugin.servlet.ExternalPluginTestSpanServlet + + ExternalPluginTestTransactionServlet + co.elastic.apm.external.plugin.servlet.ExternalPluginTestTransactionServlet + + + + ExternalPluginTestSpanServlet + /test-span-external-plugin + - ExternalPluginTestServlet - /test-external-plugin + ExternalPluginTestTransactionServlet + /test-transaction-external-plugin diff --git a/integration-tests/external-plugin-test/external-plugin-jakarta-app/pom.xml b/integration-tests/external-plugin-test/external-plugin-jakarta-app/pom.xml new file mode 100644 index 0000000000..7e98b4fac8 --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-jakarta-app/pom.xml @@ -0,0 +1,38 @@ + + + + 4.0.0 + + + external-plugin-test + co.elastic.apm + 1.28.5-SNAPSHOT + + + external-plugin-jakarta-app + ${project.groupId}:${project.artifactId} + + war + + + ${project.basedir}/../../.. + + + + external-plugin-jakarta-webapp + + + + + jakarta.servlet + jakarta.servlet-api + 5.0.0 + provided + + + ${project.groupId} + plugin-instrumentation-target + ${project.version} + + + diff --git a/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java new file mode 100644 index 0000000000..81542f156c --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java @@ -0,0 +1,52 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.external.plugin.servlet; + +import co.elastic.apm.plugin.test.TestClass; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import java.io.IOException; + +/** + * This Servlet demonstrates a full integration test through a webapp that is tested on all Servlet containers in + * `integration-tests/application-server-integration-tests` (see {@code ExternalPluginTestApp}). When tested this way, + * the plugin is loaded as a jar from the configured plugin directory, including the instrumentation class itself and + * the META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation file. + * + * Since a transaction is created through the Servlet instrumentation, the instrumented method ({@link TestClass#traceMe(boolean)}) + * invocation targeted by the external plugin should be captured as a span. + */ +public class ExternalPluginTestSpanServlet extends HttpServlet { + + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { + try { + // Using an external library type that is expected to be instrumented by the external plugin. + // We expect a transaction to be created by the general Servlet API instrumentation, and a child span + // through the plugin instrumentation of the TestClass#traceMe() method. We also expect an error that + // would not have been captured without the external plugin as the thrown Exception is quietly caught. + // See ExternalPluginTestApp in `integration-tests/application-server-integration-tests`. + new TestClass().traceMe(true); + } catch (IllegalStateException e) { + // do nothing - expected + } + response.getWriter().append("Success"); + } +} diff --git a/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java new file mode 100644 index 0000000000..be25161e99 --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java @@ -0,0 +1,53 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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. + */ +package co.elastic.apm.external.plugin.servlet; + +import co.elastic.apm.plugin.test.TestClass; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import java.io.IOException; + +/** + * This Servlet demonstrates a full integration test through a webapp that is tested on all Servlet containers in + * `integration-tests/application-server-integration-tests` (see {@code ExternalPluginTestApp}). When tested this way, + * the plugin is loaded as a jar from the configured plugin directory, including the instrumentation class itself and + * the META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation file. + * + * Since the path corresponding this Servlet is ignored, a transaction is created through the Servlet instrumentation. + * Therefore, the instrumented method ({@link TestClass#traceMe(boolean)}) invocation targeted by the external plugin + * should be captured as a transaction. + */ +public class ExternalPluginTestTransactionServlet extends HttpServlet { + + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { + try { + // Using an external library type that is expected to be instrumented by the external plugin. + // A transaction should not be created by the general Servlet API instrumentation because we ignore the + // corresponding URL, so we expect to get a transaction for the TestClass#traceMe() invocation. + // We also expect an error related to this transaction. + // See ExternalPluginTestApp in `integration-tests/application-server-integration-tests`. + new TestClass().traceMe(true); + } catch (IllegalStateException e) { + // do nothing - expected + } + response.getWriter().append("Success"); + } +} diff --git a/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/WEB-INF/web.xml b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/WEB-INF/web.xml new file mode 100644 index 0000000000..b8eab25204 --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/WEB-INF/web.xml @@ -0,0 +1,25 @@ + + + + + ExternalPluginTestSpanServlet + co.elastic.apm.external.plugin.servlet.ExternalPluginTestSpanServlet + + + ExternalPluginTestTransactionServlet + co.elastic.apm.external.plugin.servlet.ExternalPluginTestTransactionServlet + + + + ExternalPluginTestSpanServlet + /test-span-external-plugin + + + ExternalPluginTestTransactionServlet + /test-transaction-external-plugin + + + diff --git a/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/status.html b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/status.html new file mode 100644 index 0000000000..3c2aefee02 --- /dev/null +++ b/integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/status.html @@ -0,0 +1,5 @@ + + +OK + + diff --git a/integration-tests/external-plugin-test/external-plugin/src/main/java/co/elastic/apm/agent/plugin/PluginInstrumentation.java b/integration-tests/external-plugin-test/external-plugin/src/main/java/co/elastic/apm/agent/plugin/PluginInstrumentation.java index 61442cb718..f140112677 100644 --- a/integration-tests/external-plugin-test/external-plugin/src/main/java/co/elastic/apm/agent/plugin/PluginInstrumentation.java +++ b/integration-tests/external-plugin-test/external-plugin/src/main/java/co/elastic/apm/agent/plugin/PluginInstrumentation.java @@ -27,6 +27,8 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Collection; import java.util.Collections; @@ -35,6 +37,14 @@ public class PluginInstrumentation extends ElasticApmInstrumentation { + /** + * This slf4j logger relies on the slf4j that comes from the instrumented library (see {@link co.elastic.apm.plugin.test.TestClass}). + * If an external plugin contains slf4j classes, it will be rejected by the agent. + * Same for all other packages listed in {@code co.elastic.apm.agent.common.util.AgentInfo#dependencyPackages}. + * All these packages can be used only if their scope is {@code provided}. + */ + private static final Logger logger = LoggerFactory.getLogger(PluginInstrumentation.class); + @Override public ElementMatcher getTypeMatcher() { return named(System.getProperty("elastic.apm.plugin.instrumented_class", "co.elastic.apm.plugin.test.TestClass")); @@ -52,25 +62,26 @@ public Collection getInstrumentationGroupNames() { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) - public static Object onEnter(@Advice.Origin(value = "#m") String methodName) { + public static Object onEnter(@Advice.Origin Class clazz, @Advice.Origin(value = "#m") String methodName) { Span ret; Transaction transaction = ElasticApm.currentTransaction(); if (transaction.getId().isEmpty()) { // the NoopTransaction ret = ElasticApm.startTransaction(); - System.out.println("ret = " + ret); + ret.setName(clazz.getSimpleName() + "#" + methodName); } else { ret = transaction.startSpan("plugin", "external", "trace"); - System.out.println("ret = " + ret); + ret.setName(methodName); } - return ret.setName(methodName).activate(); + logger.info("ret = " + ret); + return ret.activate(); } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) public static void onExit(@Advice.Thrown Throwable thrown, @Advice.Enter Object scopeObject) { try { Span span = ElasticApm.currentSpan(); - System.out.println("span = " + span); + logger.info("span = " + span); span.captureException(thrown); span.end(); } finally { diff --git a/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java b/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java index f14aaf0661..45b6078850 100644 --- a/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java +++ b/integration-tests/external-plugin-test/external-plugin/src/test/java/co/elastic/apm/agent/plugin/PluginInstrumentationTest.java @@ -34,8 +34,8 @@ * test of the plugin, where it is loaded from a plugin directory and loaded with the proper class loader. * When running this test, the META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation file is loaded * from the system classpath and the instrumentation class is loaded as an internal plugin. In order to fully test - * the external plugin, see `integration-tests/external-plugin-app`, which creates a webapp that is tested on all - * Servlet containers in `integration-tests/application-server-integration-tests`. + * the external plugin, see `integration-tests/external-plugin-app` and `integration-tests/external-plugin-jakarta-app` + * that create webapps that are tested on all Servlet containers in `integration-tests/application-server-integration-tests`. *
* Implementation note: within this test, due to not testing a packaged external plugin, the * {@code co.elastic.apm.agent.} package prefix is required for the instrumentation class. When plugin is loaded @@ -52,7 +52,7 @@ void disableObjectRecyclingAssertion() { void testTransactionCreation() { new TestClass().traceMe(false); assertThat(reporter.getTransactions()).hasSize(1); - assertThat(reporter.getFirstTransaction().getNameAsString()).isEqualTo("traceMe"); + assertThat(reporter.getFirstTransaction().getNameAsString()).isEqualTo("TestClass#traceMe"); assertThat(reporter.getErrors()).isEmpty(); } @@ -86,7 +86,7 @@ void testErrorCreation() { } assertThat(reporter.getTransactions()).hasSize(1); Transaction transaction = reporter.getFirstTransaction(); - assertThat(transaction.getNameAsString()).isEqualTo("traceMe"); + assertThat(transaction.getNameAsString()).isEqualTo("TestClass#traceMe"); assertThat(reporter.getErrors()).hasSize(1); assertThat(reporter.getFirstError().getTraceContext().getTransactionId()).isEqualTo(transaction.getTraceContext().getId()); } diff --git a/integration-tests/external-plugin-test/plugin-instrumentation-target/pom.xml b/integration-tests/external-plugin-test/plugin-instrumentation-target/pom.xml index 0136fd049a..cfae15aa1c 100644 --- a/integration-tests/external-plugin-test/plugin-instrumentation-target/pom.xml +++ b/integration-tests/external-plugin-test/plugin-instrumentation-target/pom.xml @@ -15,4 +15,12 @@ ${project.basedir}/../../.. + + + + org.slf4j + slf4j-api + 1.7.25 + + diff --git a/integration-tests/external-plugin-test/plugin-instrumentation-target/src/main/java/co/elastic/apm/plugin/test/TestClass.java b/integration-tests/external-plugin-test/plugin-instrumentation-target/src/main/java/co/elastic/apm/plugin/test/TestClass.java index 6e16c7117d..3f0d6eac2d 100644 --- a/integration-tests/external-plugin-test/plugin-instrumentation-target/src/main/java/co/elastic/apm/plugin/test/TestClass.java +++ b/integration-tests/external-plugin-test/plugin-instrumentation-target/src/main/java/co/elastic/apm/plugin/test/TestClass.java @@ -18,9 +18,21 @@ */ package co.elastic.apm.plugin.test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + public class TestClass { + private static final Logger logger = LoggerFactory.getLogger(TestClass.class); + public void traceMe(boolean throwException) throws IllegalStateException { + + // Testing that usage of slf4j in instrumented library doesn't break anything and that log correlation works + String infoMessage = String.format("TestClass#traceMe was called, transaction ID: %s, trace ID: %s", MDC.get("transaction.id"), MDC.get("trace.id")); + logger.info(infoMessage); + System.out.println(infoMessage); + if (throwException) { throw new IllegalStateException("Test Exception"); } diff --git a/integration-tests/external-plugin-test/pom.xml b/integration-tests/external-plugin-test/pom.xml index 5a1ea9621c..3c70cac77b 100644 --- a/integration-tests/external-plugin-test/pom.xml +++ b/integration-tests/external-plugin-test/pom.xml @@ -14,6 +14,7 @@ plugin-instrumentation-target external-plugin-app + external-plugin-jakarta-app external-plugin