From 3473586c79249381dae041ea7b4572624513fe26 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Wed, 29 Dec 2021 09:07:05 +0200 Subject: [PATCH 1/6] Fix slf4j LinkageErrors for external plugins --- .github/dependabot.yml | 4 + .../elastic/apm/agent/bci/IndyBootstrap.java | 17 ++- .../bci/bytebuddy/CustomElementMatchers.java | 3 +- .../apm/agent/impl/ElasticApmTracer.java | 3 - .../agent/impl/transaction/AbstractSpan.java | 13 ++- .../agent/impl/transaction/TraceContext.java | 3 +- .../apm/agent/util/ClassLoaderUtils.java | 4 + .../apm/agent/impl/ElasticApmTracerTest.java | 2 + .../TransactionInstrumentationTest.java | 10 ++ .../opentracing/OpenTracingBridgeTest.java | 47 ++++++-- .../pom.xml | 6 + ...stractServletContainerIntegrationTest.java | 8 +- .../elastic/apm/servlet/AbstractTomcatIT.java | 2 +- .../java/co/elastic/apm/servlet/JBossIT.java | 4 +- .../elastic/apm/servlet/JakartaeeJettyIT.java | 7 +- .../apm/servlet/JakartaeeTomcatIT.java | 5 +- .../java/co/elastic/apm/servlet/JettyIT.java | 9 +- .../java/co/elastic/apm/servlet/PayaraIT.java | 5 +- .../java/co/elastic/apm/servlet/TomcatIT.java | 4 +- .../co/elastic/apm/servlet/WebLogicIT.java | 4 +- .../co/elastic/apm/servlet/WebSphereIT.java | 4 +- .../co/elastic/apm/servlet/WildFlyIT.java | 4 +- .../servlet/tests/ExternalPluginTestApp.java | 104 +++++++++++++++--- .../tests/JakartaExternalPluginTestApp.java | 26 +++++ .../tests/JavaxExternalPluginTestApp.java | 26 +++++ .../co/elastic/apm/servlet/tests/TestApp.java | 10 ++ ...ava => ExternalPluginTestSpanServlet.java} | 5 +- .../ExternalPluginTestTransactionServlet.java | 50 +++++++++ .../src/main/webapp/WEB-INF/web.xml | 17 ++- .../external-plugin-jakarta-app/pom.xml | 38 +++++++ .../ExternalPluginTestSpanServlet.java | 52 +++++++++ .../ExternalPluginTestTransactionServlet.java | 53 +++++++++ .../src/main/webapp/WEB-INF/web.xml | 25 +++++ .../src/main/webapp/status.html | 5 + .../external-plugin/pom.xml | 5 + .../agent/plugin/PluginInstrumentation.java | 15 ++- .../plugin/PluginInstrumentationTest.java | 8 +- .../plugin-instrumentation-target/pom.xml | 8 ++ .../co/elastic/apm/plugin/test/TestClass.java | 12 ++ .../external-plugin-test/pom.xml | 1 + 40 files changed, 561 insertions(+), 67 deletions(-) create mode 100644 integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaExternalPluginTestApp.java create mode 100644 integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JavaxExternalPluginTestApp.java rename integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/{ExternalPluginTestServlet.java => ExternalPluginTestSpanServlet.java} (88%) create mode 100644 integration-tests/external-plugin-test/external-plugin-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java create mode 100644 integration-tests/external-plugin-test/external-plugin-jakarta-app/pom.xml create mode 100644 integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestSpanServlet.java create mode 100644 integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/java/co/elastic/apm/external/plugin/servlet/ExternalPluginTestTransactionServlet.java create mode 100644 integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/WEB-INF/web.xml create mode 100644 integration-tests/external-plugin-test/external-plugin-jakarta-app/src/main/webapp/status.html 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/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 efb2472fb2..cb8e26702d 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,22 @@ 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")) && + // slf4j classes, if used by the plugin, should be loaded by the agent Class Loader and not the + // IndyPluginCL, which is child-first, as this would cause LinkageErrors due to clash with the agent's slf4j + !(externalPluginClass.startsWith("org.slf4j")) + ) { + 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 +393,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 ec5df5cfc6..cf5472d2ac 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/impl/ElasticApmTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java index 47411c6b00..0362199805 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 f12f28d2f3..f59ab61c6b 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,14 +390,17 @@ public boolean isExit() { return isExit; } - - public void captureException(long epochMicros, Throwable t) { - tracer.captureAndReportException(epochMicros, t, this); + @Nullable + public String captureException(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); } 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 captureException(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 83eab2732b..02a738f686 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 org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -712,7 +713,7 @@ public int hashCode() { } void setApplicationClassLoader(@Nullable ClassLoader classLoader) { - if (classLoader != null) { + if (classLoader != null && !ClassLoaderUtils.isAgentClassLoader(classLoader)) { WeakReference local = classLoaderWeakReferenceCache.get(classLoader); if (local == null) { local = new WeakReference<>(classLoader); 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..d78b59ea44 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,8 @@ 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"); + } } 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-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/integration-tests/application-server-integration-tests/pom.xml b/integration-tests/application-server-integration-tests/pom.xml index 8534b91c48..dfcb299c8f 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 5239766e75..df663c1ed3 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 @@ -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..d52ff4bd7f 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=y,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..d2258464e2 --- /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.4-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/pom.xml b/integration-tests/external-plugin-test/external-plugin/pom.xml index 76eb8a99c6..0e64b3f511 100644 --- a/integration-tests/external-plugin-test/external-plugin/pom.xml +++ b/integration-tests/external-plugin-test/external-plugin/pom.xml @@ -40,6 +40,11 @@ 3.9 --> + + org.slf4j + slf4j-api + 1.7.25 + - - org.slf4j - slf4j-api - 1.7.25 -