From da027cdd85abb72bbc9aedd7a6457ef6e790e618 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 31 Jan 2022 09:13:50 +0100 Subject: [PATCH 01/10] Service name discovery based on webapp manifest - Refactoring of servlet plugin --- .../apm/agent/configuration/ServiceInfo.java | 81 ++++-- .../apm/agent/impl/ElasticApmTracer.java | 16 +- .../elastic/apm/agent/impl/GlobalTracer.java | 10 +- .../co/elastic/apm/agent/impl/NoopTracer.java | 7 +- .../co/elastic/apm/agent/impl/Tracer.java | 18 +- .../apm/agent/impl/ElasticApmTracerTest.java | 10 +- .../serialize/MetricRegistryReporterTest.java | 3 +- .../serialize/MetricSetSerializationTest.java | 4 +- .../api/ElasticApmApiInstrumentationTest.java | 9 +- .../agent/servlet/AbstractServletTest.java | 2 +- ...ice.java => JakartaServletApiAdapter.java} | 83 +++--- .../servlet/JarkataServletApiAdvice.java | 46 +++ .../agent/servlet/JavaxServletApiAdapter.java | 273 ++++++++++++++++++ .../agent/servlet/JavaxServletApiAdvice.java | 225 +-------------- ...vletHelper.java => ServletApiAdapter.java} | 25 +- .../apm/agent/servlet/ServletApiAdvice.java | 106 ++++--- .../apm/agent/servlet/ServletGlobalState.java | 34 --- .../servlet/ServletServiceNameHelper.java | 101 +++++++ .../servlet/ServletTransactionHelper.java | 57 ++-- ...> AbstractServletRequestHeaderGetter.java} | 10 +- .../JakartaServletRequestHeaderGetter.java | 8 +- ...kartaServletTransactionCreationHelper.java | 70 ----- .../JavaxServletRequestHeaderGetter.java | 8 +- ...JavaxServletTransactionCreationHelper.java | 71 ----- .../ServletTransactionCreationHelper.java | 112 ------- .../agent/servlet/AbstractServletTest.java | 2 +- .../servlet/ServletServiceNameHelperTest.java | 117 ++++++++ .../servlet/ServletTransactionHelperTest.java | 24 -- .../servlet/helper/ServletApiAdapterTest.java | 48 +++ .../ServletTransactionCreationHelperTest.java | 34 +-- .../src/test/resources/TEST-MANIFEST.MF | 2 + .../SpringServiceNameInstrumentation.java | 7 +- .../servlet/tests/ExternalPluginTestApp.java | 3 +- 33 files changed, 844 insertions(+), 782 deletions(-) rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{JakartaServletApiAdvice.java => JakartaServletApiAdapter.java} (75%) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{ServletHelper.java => ServletApiAdapter.java} (82%) delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletGlobalState.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/{CommonServletRequestHeaderGetter.java => AbstractServletRequestHeaderGetter.java} (79%) delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletTransactionCreationHelper.java delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletTransactionCreationHelper.java delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/resources/TEST-MANIFEST.MF diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java index fafb9f41ad..c2b301d2da 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java @@ -22,13 +22,14 @@ import java.util.Properties; import java.util.jar.Attributes; import java.util.jar.JarFile; +import java.util.jar.Manifest; public class ServiceInfo { private static final String JAR_VERSION_SUFFIX = "-(\\d+\\.)+(\\d+)(.*)?$"; private static final String DEFAULT_SERVICE_NAME = "unknown-java-service"; + private static final ServiceInfo EMPTY = ServiceInfo.of(null); private static final ServiceInfo AUTO_DETECTED = autoDetect(System.getProperties()); - private final String serviceName; @Nullable private final String serviceVersion; @@ -37,7 +38,7 @@ public ServiceInfo(@Nullable String serviceName) { this(serviceName, null); } - public ServiceInfo(@Nullable String serviceName, @Nullable String serviceVersion) { + private ServiceInfo(@Nullable String serviceName, @Nullable String serviceVersion) { if (serviceName == null || serviceName.trim().isEmpty()) { this.serviceName = DEFAULT_SERVICE_NAME; } else { @@ -46,13 +47,20 @@ public ServiceInfo(@Nullable String serviceName, @Nullable String serviceVersion this.serviceVersion = serviceVersion; } - public String getServiceName() { - return serviceName; + public static ServiceInfo empty() { + return new ServiceInfo(null, null); } - @Nullable - public String getServiceVersion() { - return serviceVersion; + public static ServiceInfo of(@Nullable String serviceName) { + return of(serviceName, null); + } + + public static ServiceInfo of(@Nullable String serviceName, @Nullable String serviceVersion) { + if ((serviceName == null || serviceName.isEmpty()) && + (serviceVersion == null || serviceVersion.isEmpty())) { + return ServiceInfo.empty(); + } + return new ServiceInfo(serviceName, serviceVersion); } public static String replaceDisallowedServiceNameChars(String serviceName) { @@ -72,7 +80,7 @@ public static ServiceInfo autoDetect(Properties properties) { if (serviceInfo != null) { return serviceInfo; } - return new ServiceInfo(null); + return ServiceInfo.empty(); } } @@ -84,12 +92,12 @@ private static ServiceInfo createFromSunJavaCommand(@Nullable String command) { command = command.trim(); String serviceName = getContainerServiceName(command); if (serviceName != null) { - return new ServiceInfo(serviceName); + return ServiceInfo.of(serviceName); } if (command.contains(".jar")) { - return parseJarCommand(command); + return fromJarCommand(command); } else { - return parseMainClass(command); + return fromMainClassCommand(command); } } @@ -111,26 +119,32 @@ private static String getContainerServiceName(String command) { return null; } - private static ServiceInfo parseJarCommand(String command) { + private static ServiceInfo fromJarCommand(String command) { final String[] commandParts = command.split(" "); - String serviceName = null; - String serviceVersion = null; + ServiceInfo serviceInfoFromManifest = ServiceInfo.empty(); + ServiceInfo serviceInfoFromJarName = ServiceInfo.empty(); for (String commandPart : commandParts) { if (commandPart.endsWith(".jar")) { try (JarFile jarFile = new JarFile(commandPart)) { - Attributes mainAttributes = jarFile.getManifest().getMainAttributes(); - serviceName = mainAttributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE); - serviceVersion = mainAttributes.getValue(Attributes.Name.IMPLEMENTATION_VERSION); + serviceInfoFromManifest = fromManifest(jarFile.getManifest()); } catch (Exception ignored) { } - if (serviceName == null || serviceName.isEmpty()) { - serviceName = removeVersionFromJar(removePath(removeJarExtension(commandPart))); - } + serviceInfoFromJarName = ServiceInfo.of(removeVersionFromJar(removePath(removeJarExtension(commandPart)))); break; } } - return new ServiceInfo(serviceName, serviceVersion); + return serviceInfoFromManifest.withFallback(serviceInfoFromJarName); + } + + public static ServiceInfo fromManifest(@Nullable Manifest manifest) { + if (manifest == null) { + return ServiceInfo.empty(); + } + Attributes mainAttributes = manifest.getMainAttributes(); + return ServiceInfo.of( + mainAttributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE), + mainAttributes.getValue(Attributes.Name.IMPLEMENTATION_VERSION)); } private static String removeJarExtension(String commandPart) { @@ -145,7 +159,7 @@ private static String removeVersionFromJar(String jarFileName) { return jarFileName.replaceFirst(JAR_VERSION_SUFFIX, ""); } - private static ServiceInfo parseMainClass(String command) { + private static ServiceInfo fromMainClassCommand(String command) { final String mainClassName; int indexOfSpace = command.indexOf(' '); if (indexOfSpace != -1) { @@ -155,4 +169,27 @@ private static ServiceInfo parseMainClass(String command) { } return new ServiceInfo(mainClassName.substring(mainClassName.lastIndexOf('.') + 1)); } + + public String getServiceName() { + return serviceName; + } + + @Nullable + public String getServiceVersion() { + return serviceVersion; + } + + public ServiceInfo withFallback(ServiceInfo fallback) { + return ServiceInfo.of( + hasServiceName() ? serviceName : fallback.serviceName, + serviceVersion != null ? serviceVersion : fallback.serviceVersion); + } + + public boolean hasServiceName() { + return !serviceName.equals(DEFAULT_SERVICE_NAME); + } + + public boolean isEmpty() { + return !hasServiceName() && serviceVersion == null; + } } 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 50faae45c7..e1d6557bb5 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 @@ -41,12 +41,12 @@ import co.elastic.apm.agent.report.ApmServerClient; import co.elastic.apm.agent.report.Reporter; import co.elastic.apm.agent.report.ReporterConfiguration; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; import co.elastic.apm.agent.util.DependencyInjectingServiceLoader; import co.elastic.apm.agent.util.ExecutorUtils; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; import org.stagemonitor.configuration.ConfigurationOption; import org.stagemonitor.configuration.ConfigurationOptionProvider; import org.stagemonitor.configuration.ConfigurationRegistry; @@ -748,25 +748,19 @@ public List getServiceInfoOverrides() { } @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName) { - overrideServiceInfoForClassLoader(classLoader, serviceName, null); - } - - @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName, @Nullable String serviceVersion) { + public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, ServiceInfo serviceInfo) { // overriding the service name/version for the bootstrap class loader is not an actual use-case // null may also mean we don't know about the initiating class loader if (classLoader == null - || serviceName == null || serviceName.isEmpty() + || !serviceInfo.hasServiceName() // if the service name is set explicitly, don't override it || coreConfiguration.getServiceNameConfig().getUsedKey() != null) { return; } - ServiceInfo serviceInfo = new ServiceInfo(serviceName, serviceVersion); logger.debug("Using `{}` as the service name and `{}` as the service version for class loader [{}]", serviceInfo.getServiceName(), serviceInfo.getServiceVersion(), classLoader); if (!serviceInfoByClassLoader.containsKey(classLoader)) { - serviceInfoByClassLoader.putIfAbsent(classLoader, new ServiceInfo(serviceName, serviceVersion)); + serviceInfoByClassLoader.putIfAbsent(classLoader, serviceInfo); } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java index 344769aca9..c7b72ea4fc 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.impl; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.impl.transaction.AbstractSpan; @@ -204,13 +205,8 @@ public TracerState getState() { } @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName) { - tracer.overrideServiceInfoForClassLoader(classLoader, serviceName); - } - - @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName, @Nullable String serviceVersion) { - tracer.overrideServiceInfoForClassLoader(classLoader, serviceName, serviceVersion); + public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, ServiceInfo serviceInfo) { + tracer.overrideServiceInfoForClassLoader(classLoader, serviceInfo); } @Override diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/NoopTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/NoopTracer.java index 09dab82d30..4621a45b18 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/NoopTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/NoopTracer.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.impl; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.impl.transaction.AbstractSpan; @@ -130,11 +131,7 @@ public TracerState getState() { } @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName) { - } - - @Override - public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName, @Nullable String serviceVersion) { + public void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, ServiceInfo serviceInfo) { } @Override diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/Tracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/Tracer.java index 43c8b6b9aa..ade46e7c7a 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/Tracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/Tracer.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.impl; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.impl.transaction.AbstractSpan; @@ -152,18 +153,6 @@ Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHeaderGet TracerState getState(); - /** - * Overrides the service name for all {@link Transaction}s, - * {@link Span}s and {@link ErrorCapture}s which are created by the service which corresponds to the provided {@link ClassLoader}. - *

- * The main use case is being able to differentiate between multiple services deployed to the same application server. - *

- * - * @param classLoader the class loader which corresponds to a particular service - * @param serviceName the service name for this class loader - */ - void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName); - /** * Overrides the service name and version for all {@link Transaction}s, * {@link Span}s and {@link ErrorCapture}s which are created by the service which corresponds to the provided {@link ClassLoader}. @@ -172,10 +161,9 @@ Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHeaderGet *

* * @param classLoader the class loader which corresponds to a particular service - * @param serviceName the service name for this class loader - * @param serviceVersion the service version for this class loader + * @param serviceInfo the service name and version for this class loader */ - void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, @Nullable String serviceName, @Nullable String serviceVersion); + void overrideServiceInfoForClassLoader(@Nullable ClassLoader classLoader, ServiceInfo serviceInfo); /** * Called when the container shuts down. 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 3e650650ce..a0b7040af9 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 @@ -431,7 +431,7 @@ void testOverrideServiceNameWithoutExplicitServiceName() { .configurationRegistry(SpyConfiguration.createSpyConfig()) .reporter(reporter) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), "overridden"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden")); startTestRootTransaction().end(); @@ -448,7 +448,7 @@ void testNotOverrideServiceNameWhenServiceNameConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), "overridden"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden")); startTestRootTransaction().end(); @@ -467,7 +467,7 @@ void testNotOverrideServiceNameWhenDefaultServiceNameConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), "overridden"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden", null)); startTestRootTransaction().end(); CoreConfiguration coreConfig = localConfig.getConfig(CoreConfiguration.class); @@ -485,7 +485,7 @@ void testOverrideServiceVersionWithoutExplicitServiceVersion() { final ElasticApmTracer tracer = new ElasticApmTracerBuilder() .reporter(reporter) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), "overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); startTestRootTransaction().end(); @@ -500,7 +500,7 @@ void testNotOverrideServiceVersionWhenServiceVersionConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), "overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); startTestRootTransaction().end(); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricRegistryReporterTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricRegistryReporterTest.java index adc3a10b42..d965364c59 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricRegistryReporterTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricRegistryReporterTest.java @@ -19,6 +19,7 @@ package co.elastic.apm.agent.report.serialize; import co.elastic.apm.agent.MockReporter; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.configuration.SpyConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.ElasticApmTracerBuilder; @@ -40,7 +41,7 @@ void testReportedMetricsUseDefaultServiceNameIfServiceNameIsExplicitlySet() thro .configurationRegistry(SpyConfiguration.createSpyConfig(SimpleSource.forTest("service_name", "foo"))) .reporter(reporter) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(MetricRegistryReporterTest.class.getClassLoader(), "MetricRegistryReporterTest"); + tracer.overrideServiceInfoForClassLoader(MetricRegistryReporterTest.class.getClassLoader(), ServiceInfo.of("MetricRegistryReporterTest")); new MetricRegistryReporter(tracer).run(); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricSetSerializationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricSetSerializationTest.java index e9c7f52083..6e679f3806 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricSetSerializationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/MetricSetSerializationTest.java @@ -206,7 +206,7 @@ void testServiceNameAndVersion() throws Exception { void testServiceNameOverrideWithOneService() throws Exception { registry.updateTimer("foo", Labels.Mutable.of(), 1); - JsonNode jsonNode = reportAsJson(singletonList(new ServiceInfo("bar", "1.0"))); + JsonNode jsonNode = reportAsJson(singletonList(ServiceInfo.of("bar", "1.0"))); assertThat(jsonNode).isNotNull(); JsonNode serviceName = jsonNode.get("metricset").get("service").get("name"); assertThat(serviceName.asText()).isEqualTo("bar"); @@ -222,7 +222,7 @@ void testServiceNameOverrideWithMultipleService() throws Exception { registry.flipPhaseAndReport( metricSets -> jwFuture.complete(metricRegistrySerializer.serialize( metricSets.values().iterator().next(), - List.of(new ServiceInfo("bar1", "2.0"), new ServiceInfo("bar2", null)) + List.of(ServiceInfo.of("bar1", "2.0"), ServiceInfo.of("bar2")) )) ); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index db9c6af16f..5bba9f49a0 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -19,6 +19,7 @@ package co.elastic.apm.api; import co.elastic.apm.AbstractApiTest; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.impl.TextHeaderMapAccessor; import co.elastic.apm.agent.impl.TracerInternalApiUtils; @@ -327,21 +328,21 @@ void testManualTimestampsDeactivated() { @Test void testOverrideServiceNameForClassLoader() { - tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), "overridden"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden")); ElasticApm.startTransaction().end(); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden"); } @Test void testOverrideServiceNameForClassLoaderWithRemoteParent() { - tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), "overridden"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden")); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden"); } @Test void testOverrideServiceVersionForClassLoader() { - tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), "overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); ElasticApm.startTransaction().end(); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); @@ -349,7 +350,7 @@ void testOverrideServiceVersionForClassLoader() { @Test void testOverrideServiceVersionForClassLoaderWithRemoteParent() { - tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), "overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); diff --git a/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java b/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java index ba16a36b42..52b405e3a8 100644 --- a/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java +++ b/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java @@ -46,7 +46,7 @@ abstract class AbstractServletTest extends AbstractInstrumentationTest { void initServerAndClient() throws Exception { // because we reuse the same classloader with different servlet context names // we need to explicitly reset the name cache to make service name detection work as expected - ServletGlobalState.clearServiceNameCache(); + ServletServiceNameHelper.clearServiceNameCache(); // server is not reused between tests as handler is provided from subclass // another alternative diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java similarity index 75% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java index 8bf60749c0..7f05124fe1 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java @@ -18,10 +18,9 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.impl.GlobalTracer; import co.elastic.apm.agent.impl.context.Request; -import co.elastic.apm.agent.impl.transaction.Transaction; -import co.elastic.apm.agent.servlet.helper.JakartaServletTransactionCreationHelper; +import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; +import co.elastic.apm.agent.servlet.helper.JakartaServletRequestHeaderGetter; import jakarta.servlet.DispatcherType; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContext; @@ -31,48 +30,41 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import net.bytebuddy.asm.Advice; import javax.annotation.Nullable; +import java.io.InputStream; import java.security.Principal; import java.util.Collection; import java.util.Enumeration; import java.util.Map; -public class JakartaServletApiAdvice extends ServletApiAdvice implements ServletHelper { +public class JakartaServletApiAdapter implements ServletApiAdapter { - private static JakartaServletTransactionCreationHelper transactionCreationHelper; - private static JakartaServletApiAdvice helper; + public static final JakartaServletApiAdapter INSTANCE = new JakartaServletApiAdapter(); - static { - transactionCreationHelper = new JakartaServletTransactionCreationHelper(GlobalTracer.requireTracerImpl()); - helper = new JakartaServletApiAdvice(); + private JakartaServletApiAdapter() { } - @Nullable - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) - public static Object onEnterServletService(@Advice.Argument(0) ServletRequest servletRequest) { - return onServletEnter(servletRequest, helper); - } - - - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onExitServletService(@Advice.Argument(0) ServletRequest servletRequest, - @Advice.Argument(1) ServletResponse servletResponse, - @Advice.Enter @Nullable Object transactionOrScopeOrSpan, - @Advice.Thrown @Nullable Throwable t, - @Advice.This Object thiz) { - onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, helper); + public static JakartaServletApiAdapter get() { + return INSTANCE; } + @Nullable @Override - public boolean isHttpServletRequest(ServletRequest servletRequest) { - return servletRequest instanceof HttpServletRequest; + public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { + if (servletRequest instanceof HttpServletRequest) { + return (HttpServletRequest) servletRequest; + } + return null; } + @Nullable @Override - public boolean isHttpServletResponse(ServletResponse servletResponse) { - return servletResponse instanceof HttpServletResponse; + public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse) { + if (servletResponse instanceof HttpServletResponse) { + return (HttpServletResponse) servletResponse; + } + return null; } @Override @@ -100,9 +92,21 @@ public boolean isErrorDispatcherType(ServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.ERROR; } + @Nullable @Override - public ClassLoader getClassloader(ServletContext servletContext) { - return transactionCreationHelper.getClassloader(servletContext); + public ClassLoader getClassLoader(@Nullable ServletContext servletContext) { + if (servletContext == null) { + return null; + } + + // getClassloader might throw UnsupportedOperationException + // see Section 4.4 of the Servlet 3.0 specification + try { + return servletContext.getClassLoader(); + } catch (UnsupportedOperationException ignore) { + // silently ignored + return null; + } } @Override @@ -110,16 +114,12 @@ public String getServletContextName(ServletContext servletContext) { return servletContext.getServletContextName(); } + @Nullable @Override public String getContextPath(ServletContext servletContext) { return servletContext.getContextPath(); } - @Override - public Transaction createAndActivateTransaction(HttpServletRequest httpServletRequest) { - return transactionCreationHelper.createAndActivateTransaction(httpServletRequest); - } - @Override public void handleCookies(Request request, HttpServletRequest servletRequest) { if (servletRequest.getCookies() != null) { @@ -256,7 +256,18 @@ public int getStatus(HttpServletResponse httpServletResponse) { } @Override - public ServletContext getServletContext(ServletRequest servletRequest) { + public ServletContext getServletContext(HttpServletRequest servletRequest) { return servletRequest.getServletContext(); } + + @Nullable + @Override + public InputStream getResourceAsStream(ServletContext servletContext, String path) { + return servletContext.getResourceAsStream(path); + } + + @Override + public TextHeaderGetter getRequestHeaderGetter() { + return JakartaServletRequestHeaderGetter.getInstance(); + } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java new file mode 100644 index 0000000000..767726bbed --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java @@ -0,0 +1,46 @@ +/* + * 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.servlet; + +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import net.bytebuddy.asm.Advice; + +import javax.annotation.Nullable; + +public class JarkataServletApiAdvice extends ServletApiAdvice { + + private static final JakartaServletApiAdapter helper = JakartaServletApiAdapter.get(); + + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static Object onEnterServletService(@Advice.Argument(0) ServletRequest servletRequest) { + return onServletEnter(helper, servletRequest); + } + + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) + public static void onExitServletService(@Advice.Argument(0) ServletRequest servletRequest, + @Advice.Argument(1) ServletResponse servletResponse, + @Advice.Enter @Nullable Object transactionOrScopeOrSpan, + @Advice.Thrown @Nullable Throwable t, + @Advice.This Object thiz) { + onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, helper); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java new file mode 100644 index 0000000000..cf428cede3 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java @@ -0,0 +1,273 @@ +/* + * 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.servlet; + +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; +import co.elastic.apm.agent.servlet.helper.JavaxServletRequestHeaderGetter; + +import javax.annotation.Nullable; +import javax.servlet.DispatcherType; +import javax.servlet.RequestDispatcher; +import javax.servlet.ServletContext; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.InputStream; +import java.security.Principal; +import java.util.Collection; +import java.util.Enumeration; +import java.util.Map; + +public class JavaxServletApiAdapter implements ServletApiAdapter { + + private static final JavaxServletApiAdapter INSTANCE = new JavaxServletApiAdapter(); + + private JavaxServletApiAdapter() { + } + + public static JavaxServletApiAdapter get() { + return INSTANCE; + } + + @Override + public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { + if (servletRequest instanceof HttpServletRequest) { + return (HttpServletRequest) servletRequest; + } + return null; + } + + @Nullable + @Override + public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse) { + if (servletResponse instanceof HttpServletResponse) { + return (HttpServletResponse) servletResponse; + } + return null; + } + + @Override + public boolean isRequestDispatcherType(ServletRequest servletRequest) { + return servletRequest.getDispatcherType() == DispatcherType.REQUEST; + } + + @Override + public boolean isAsyncDispatcherType(ServletRequest servletRequest) { + return servletRequest.getDispatcherType() == DispatcherType.ASYNC; + } + + @Override + public boolean isForwardDispatcherType(ServletRequest servletRequest) { + return servletRequest.getDispatcherType() == DispatcherType.FORWARD; + } + + @Override + public boolean isIncludeDispatcherType(ServletRequest servletRequest) { + return servletRequest.getDispatcherType() == DispatcherType.INCLUDE; + } + + @Override + public boolean isErrorDispatcherType(ServletRequest servletRequest) { + return servletRequest.getDispatcherType() == DispatcherType.ERROR; + } + + @Nullable + @Override + public ClassLoader getClassLoader(@Nullable ServletContext servletContext) { + if (servletContext == null) { + return null; + } + + // getClassloader might throw UnsupportedOperationException + // see Section 4.4 of the Servlet 3.0 specification + try { + return servletContext.getClassLoader(); + } catch (UnsupportedOperationException ignore) { + // silently ignored + return null; + } + } + + @Override + public String getServletContextName(ServletContext servletContext) { + return servletContext.getServletContextName(); + } + + @Nullable + @Override + public String getContextPath(ServletContext servletContext) { + return servletContext.getContextPath(); + } + + @Override + public void handleCookies(Request request, HttpServletRequest servletRequest) { + if (servletRequest.getCookies() != null) { + for (Cookie cookie : servletRequest.getCookies()) { + request.addCookie(cookie.getName(), cookie.getValue()); + } + } + } + + @Override + public Enumeration getRequestHeaderNames(HttpServletRequest servletRequest) { + return servletRequest.getHeaderNames(); + } + + @Override + public Enumeration getRequestHeaders(HttpServletRequest servletRequest, String name) { + return servletRequest.getHeaders(name); + } + + @Override + public String getHeader(HttpServletRequest httpServletRequest, String name) { + return httpServletRequest.getHeader(name); + } + + @Override + public String getProtocol(HttpServletRequest servletRequest) { + return servletRequest.getProtocol(); + } + + @Override + public String getMethod(HttpServletRequest servletRequest) { + return servletRequest.getMethod(); + } + + @Override + public boolean isSecure(HttpServletRequest servletRequest) { + return servletRequest.isSecure(); + } + + @Override + public String getScheme(HttpServletRequest servletRequest) { + return servletRequest.getScheme(); + } + + @Override + public String getServerName(HttpServletRequest servletRequest) { + return servletRequest.getServerName(); + } + + @Override + public int getServerPort(HttpServletRequest servletRequest) { + return servletRequest.getServerPort(); + } + + @Override + public String getRequestURI(HttpServletRequest servletRequest) { + return servletRequest.getRequestURI(); + } + + @Override + public String getQueryString(HttpServletRequest servletRequest) { + return servletRequest.getQueryString(); + } + + @Override + public String getRemoteAddr(HttpServletRequest servletRequest) { + return servletRequest.getRemoteAddr(); + } + + @Override + public String getServletPath(HttpServletRequest servletRequest) { + return servletRequest.getServletPath(); + } + + @Override + public String getPathInfo(HttpServletRequest servletRequest) { + return servletRequest.getPathInfo(); + } + + @Override + public Object getIncludeServletPathAttribute(HttpServletRequest servletRequest) { + return servletRequest.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + } + + @Override + public Object getIncludePathInfoAttribute(HttpServletRequest servletRequest) { + return servletRequest.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO); + } + + @Override + public boolean isInstanceOfHttpServlet(Object object) { + return object instanceof HttpServlet; + } + + @Override + public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { + return httpServletRequest.getUserPrincipal(); + } + + @Nullable + @Override + public Object getAttribute(ServletRequest servletRequest, String attributeName) { + return servletRequest.getAttribute(attributeName); + } + + @Nullable + @Override + public Object getHttpAttribute(HttpServletRequest httpServletRequest, String attributeName) { + return httpServletRequest.getAttribute(attributeName); + } + + @Override + public Collection getHeaderNames(HttpServletResponse httpServletResponse) { + return httpServletResponse.getHeaderNames(); + } + + @Override + public Collection getHeaders(HttpServletResponse httpServletResponse, String headerName) { + return httpServletResponse.getHeaders(headerName); + } + + @Override + public Map getParameterMap(HttpServletRequest httpServletRequest) { + return httpServletRequest.getParameterMap(); + } + + @Override + public boolean isCommitted(HttpServletResponse httpServletResponse) { + return httpServletResponse.isCommitted(); + } + + @Override + public int getStatus(HttpServletResponse httpServletResponse) { + return httpServletResponse.getStatus(); + } + + @Override + public ServletContext getServletContext(HttpServletRequest servletRequest) { + return servletRequest.getServletContext(); + } + + @Nullable + @Override + public InputStream getResourceAsStream(ServletContext servletContext, String path) { + return servletContext.getResourceAsStream(path); + } + + @Override + public TextHeaderGetter getRequestHeaderGetter() { + return JavaxServletRequestHeaderGetter.getInstance(); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java index 114b73512b..938b6fdac9 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java @@ -18,41 +18,20 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.impl.GlobalTracer; -import co.elastic.apm.agent.impl.context.Request; -import co.elastic.apm.agent.impl.transaction.Transaction; -import co.elastic.apm.agent.servlet.helper.JavaxServletTransactionCreationHelper; import net.bytebuddy.asm.Advice; import javax.annotation.Nullable; -import javax.servlet.DispatcherType; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletContext; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.security.Principal; -import java.util.Collection; -import java.util.Enumeration; -import java.util.Map; +public class JavaxServletApiAdvice extends ServletApiAdvice { -public class JavaxServletApiAdvice extends ServletApiAdvice implements ServletHelper { - - private static JavaxServletTransactionCreationHelper transactionCreationHelper; - private static JavaxServletApiAdvice helper; - static { - transactionCreationHelper = new JavaxServletTransactionCreationHelper(GlobalTracer.requireTracerImpl()); - helper = new JavaxServletApiAdvice(); - } + private static final JavaxServletApiAdapter adapter = JavaxServletApiAdapter.get(); @Nullable @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static Object onEnterServletService(@Advice.Argument(0) ServletRequest servletRequest) { - return onServletEnter(servletRequest, helper); + return onServletEnter(adapter, servletRequest); } @@ -62,202 +41,6 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl @Advice.Enter @Nullable Object transactionOrScopeOrSpan, @Advice.Thrown @Nullable Throwable t, @Advice.This Object thiz) { - onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, helper); - } - - @Override - public boolean isHttpServletRequest(ServletRequest servletRequest) { - return servletRequest instanceof HttpServletRequest; - } - - @Override - public boolean isHttpServletResponse(ServletResponse servletResponse) { - return servletResponse instanceof HttpServletResponse; - } - - @Override - public boolean isRequestDispatcherType(ServletRequest servletRequest) { - return servletRequest.getDispatcherType() == DispatcherType.REQUEST; - } - - @Override - public boolean isAsyncDispatcherType(ServletRequest servletRequest) { - return servletRequest.getDispatcherType() == DispatcherType.ASYNC; - } - - @Override - public boolean isForwardDispatcherType(ServletRequest servletRequest) { - return servletRequest.getDispatcherType() == DispatcherType.FORWARD; - } - - @Override - public boolean isIncludeDispatcherType(ServletRequest servletRequest) { - return servletRequest.getDispatcherType() == DispatcherType.INCLUDE; - } - - @Override - public boolean isErrorDispatcherType(ServletRequest servletRequest) { - return servletRequest.getDispatcherType() == DispatcherType.ERROR; - } - - @Override - public ClassLoader getClassloader(ServletContext servletContext) { - return transactionCreationHelper.getClassloader(servletContext); - } - - @Override - public String getServletContextName(ServletContext servletContext) { - return servletContext.getServletContextName(); - } - - @Override - public String getContextPath(ServletContext servletContext) { - return servletContext.getContextPath(); - } - - @Override - public Transaction createAndActivateTransaction(HttpServletRequest httpServletRequest) { - return transactionCreationHelper.createAndActivateTransaction(httpServletRequest); - } - - @Override - public void handleCookies(Request request, HttpServletRequest servletRequest) { - if (servletRequest.getCookies() != null) { - for (Cookie cookie : servletRequest.getCookies()) { - request.addCookie(cookie.getName(), cookie.getValue()); - } - } - } - - @Override - public Enumeration getRequestHeaderNames(HttpServletRequest servletRequest) { - return servletRequest.getHeaderNames(); - } - - @Override - public Enumeration getRequestHeaders(HttpServletRequest servletRequest, String name) { - return servletRequest.getHeaders(name); - } - - @Override - public String getHeader(HttpServletRequest httpServletRequest, String name) { - return httpServletRequest.getHeader(name); - } - - @Override - public String getProtocol(HttpServletRequest servletRequest) { - return servletRequest.getProtocol(); - } - - @Override - public String getMethod(HttpServletRequest servletRequest) { - return servletRequest.getMethod(); - } - - @Override - public boolean isSecure(HttpServletRequest servletRequest) { - return servletRequest.isSecure(); - } - - @Override - public String getScheme(HttpServletRequest servletRequest) { - return servletRequest.getScheme(); - } - - @Override - public String getServerName(HttpServletRequest servletRequest) { - return servletRequest.getServerName(); - } - - @Override - public int getServerPort(HttpServletRequest servletRequest) { - return servletRequest.getServerPort(); - } - - @Override - public String getRequestURI(HttpServletRequest servletRequest) { - return servletRequest.getRequestURI(); - } - - @Override - public String getQueryString(HttpServletRequest servletRequest) { - return servletRequest.getQueryString(); - } - - @Override - public String getRemoteAddr(HttpServletRequest servletRequest) { - return servletRequest.getRemoteAddr(); - } - - @Override - public String getServletPath(HttpServletRequest servletRequest) { - return servletRequest.getServletPath(); - } - - @Override - public String getPathInfo(HttpServletRequest servletRequest) { - return servletRequest.getPathInfo(); - } - - @Override - public Object getIncludeServletPathAttribute(HttpServletRequest servletRequest) { - return servletRequest.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); - } - - @Override - public Object getIncludePathInfoAttribute(HttpServletRequest servletRequest) { - return servletRequest.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO); - } - - @Override - public boolean isInstanceOfHttpServlet(Object object) { - return object instanceof HttpServlet; - } - - @Override - public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { - return httpServletRequest.getUserPrincipal(); - } - - @Nullable - @Override - public Object getAttribute(ServletRequest servletRequest, String attributeName) { - return servletRequest.getAttribute(attributeName); - } - - @Nullable - @Override - public Object getHttpAttribute(HttpServletRequest httpServletRequest, String attributeName) { - return httpServletRequest.getAttribute(attributeName); - } - - @Override - public Collection getHeaderNames(HttpServletResponse httpServletResponse) { - return httpServletResponse.getHeaderNames(); - } - - @Override - public Collection getHeaders(HttpServletResponse httpServletResponse, String headerName) { - return httpServletResponse.getHeaders(headerName); - } - - @Override - public Map getParameterMap(HttpServletRequest httpServletRequest) { - return httpServletRequest.getParameterMap(); - } - - @Override - public boolean isCommitted(HttpServletResponse httpServletResponse) { - return httpServletResponse.isCommitted(); - } - - @Override - public int getStatus(HttpServletResponse httpServletResponse) { - return httpServletResponse.getStatus(); - } - - @Override - public ServletContext getServletContext(ServletRequest servletRequest) { - return servletRequest.getServletContext(); + onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, adapter); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java similarity index 82% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletHelper.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java index 158e0c7c4d..3974b5dfcb 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java @@ -19,19 +19,22 @@ package co.elastic.apm.agent.servlet; import co.elastic.apm.agent.impl.context.Request; -import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import javax.annotation.Nullable; +import java.io.InputStream; import java.security.Principal; import java.util.Collection; import java.util.Enumeration; import java.util.Map; -public interface ServletHelper { +public interface ServletApiAdapter { - boolean isHttpServletRequest(ServletRequest servletRequest); + @Nullable + HttpServletRequest asHttpServletRequest(ServletRequest servletRequest); - boolean isHttpServletResponse(ServletResponse servletResponse); + @Nullable + HttpServletResponse asHttpServletResponse(ServletResponse servletResponse); boolean isRequestDispatcherType(ServletRequest servletRequest); @@ -44,16 +47,15 @@ public interface ServletHelper getRequestHeaderGetter(); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index b9ad3b2eab..642482aee8 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -42,18 +42,13 @@ import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_LOW_LEVEL_FRAMEWORK; import static co.elastic.apm.agent.servlet.ServletTransactionHelper.TRANSACTION_ATTRIBUTE; -import static co.elastic.apm.agent.servlet.ServletTransactionHelper.determineServiceName; public abstract class ServletApiAdvice { private static final String FRAMEWORK_NAME = "Servlet API"; static final String SPAN_TYPE = "servlet"; static final String SPAN_SUBTYPE = "request-dispatcher"; - private static final ServletTransactionHelper servletTransactionHelper; - - static { - servletTransactionHelper = new ServletTransactionHelper(GlobalTracer.requireTracerImpl()); - } + private static final ServletTransactionHelper servletTransactionHelper = new ServletTransactionHelper(GlobalTracer.requireTracerImpl()); private static final DetachedThreadLocal excluded = GlobalVariables.get(ServletApiAdvice.class, "excluded", WeakConcurrent.buildThreadLocal()); private static final DetachedThreadLocal servletPathTL = GlobalVariables.get(ServletApiAdvice.class, "servletPath", WeakConcurrent.buildThreadLocal()); @@ -62,38 +57,36 @@ public abstract class ServletApiAdvice { private static final List requestExceptionAttributes = Arrays.asList("javax.servlet.error.exception", "jakarta.servlet.error.exception", "exception", "org.springframework.web.servlet.DispatcherServlet.EXCEPTION", "co.elastic.apm.exception"); @Nullable - public static Object onServletEnter(REQUEST servletRequest, ServletHelper helper) { + public static Object onServletEnter( + ServletApiAdapter adapter, + ServletRequest servletRequest) { + ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); if (tracer == null) { return null; } AbstractSpan ret = null; // re-activate transactions for async requests - final Transaction transactionAttr = (Transaction) helper.getAttribute(servletRequest, TRANSACTION_ATTRIBUTE); + final Transaction transactionAttr = (Transaction) adapter.getAttribute(servletRequest, TRANSACTION_ATTRIBUTE); if (tracer.currentTransaction() == null && transactionAttr != null) { return transactionAttr.activateInScope(); } - if (!tracer.isRunning() || !helper.isHttpServletRequest(servletRequest)) { + final HttpServletRequest httpServletRequest = adapter.asHttpServletRequest(servletRequest); + if (!tracer.isRunning() || httpServletRequest == null) { return null; } - final HTTPREQUEST httpServletRequest = (HTTPREQUEST) servletRequest; CoreConfiguration coreConfig = tracer.getConfig(CoreConfiguration.class); - if (helper.isRequestDispatcherType(servletRequest)) { + if (adapter.isRequestDispatcherType(servletRequest)) { if (Boolean.TRUE == excluded.get()) { return null; } - CONTEXT servletContext = helper.getServletContext(servletRequest); - if (servletContext != null) { - ClassLoader servletCL = helper.getClassloader(servletContext); - // this makes sure service name discovery also works when attaching at runtime - determineServiceName(helper.getServletContextName(servletContext), servletCL, helper.getContextPath(servletContext)); - } + ServletServiceNameHelper.determineServiceName(adapter, httpServletRequest, tracer); - Transaction transaction = helper.createAndActivateTransaction(httpServletRequest); + Transaction transaction = servletTransactionHelper.createAndActivateTransaction(adapter, httpServletRequest); if (transaction == null) { // if the httpServletRequest is excluded, avoid matching all exclude patterns again on each filter invocation @@ -103,40 +96,40 @@ public static Object onS final Request req = transaction.getContext().getRequest(); if (transaction.isSampled() && coreConfig.isCaptureHeaders()) { - helper.handleCookies(req, httpServletRequest); + adapter.handleCookies(req, httpServletRequest); - final Enumeration headerNames = helper.getRequestHeaderNames(httpServletRequest); + final Enumeration headerNames = adapter.getRequestHeaderNames(httpServletRequest); if (headerNames != null) { while (headerNames.hasMoreElements()) { final String headerName = headerNames.nextElement(); - req.addHeader(headerName, helper.getRequestHeaders(httpServletRequest, headerName)); + req.addHeader(headerName, adapter.getRequestHeaders(httpServletRequest, headerName)); } } } transaction.setFrameworkName(FRAMEWORK_NAME); - servletTransactionHelper.fillRequestContext(transaction, helper.getProtocol(httpServletRequest), helper.getMethod(httpServletRequest), helper.isSecure(httpServletRequest), - helper.getScheme(httpServletRequest), helper.getServerName(httpServletRequest), helper.getServerPort(httpServletRequest), helper.getRequestURI(httpServletRequest), helper.getQueryString(httpServletRequest), - helper.getRemoteAddr(httpServletRequest), helper.getHeader(httpServletRequest, "Content-Type")); + servletTransactionHelper.fillRequestContext(transaction, adapter.getProtocol(httpServletRequest), adapter.getMethod(httpServletRequest), adapter.isSecure(httpServletRequest), + adapter.getScheme(httpServletRequest), adapter.getServerName(httpServletRequest), adapter.getServerPort(httpServletRequest), adapter.getRequestURI(httpServletRequest), adapter.getQueryString(httpServletRequest), + adapter.getRemoteAddr(httpServletRequest), adapter.getHeader(httpServletRequest, "Content-Type")); ret = transaction; - } else if (!helper.isAsyncDispatcherType(servletRequest) && coreConfig.isInstrumentationEnabled(Constants.SERVLET_API_DISPATCH)) { + } else if (!adapter.isAsyncDispatcherType(servletRequest) && coreConfig.isInstrumentationEnabled(Constants.SERVLET_API_DISPATCH)) { final AbstractSpan parent = tracer.getActive(); if (parent != null) { Object servletPath = null; Object pathInfo = null; RequestDispatcherSpanType spanType = null; - if (helper.isForwardDispatcherType(servletRequest)) { + if (adapter.isForwardDispatcherType(servletRequest)) { spanType = RequestDispatcherSpanType.FORWARD; - servletPath = helper.getServletPath(httpServletRequest); - pathInfo = helper.getPathInfo(httpServletRequest); - } else if (helper.isIncludeDispatcherType(servletRequest)) { + servletPath = adapter.getServletPath(httpServletRequest); + pathInfo = adapter.getPathInfo(httpServletRequest); + } else if (adapter.isIncludeDispatcherType(servletRequest)) { spanType = RequestDispatcherSpanType.INCLUDE; - servletPath = helper.getIncludeServletPathAttribute(httpServletRequest); - pathInfo = helper.getIncludePathInfoAttribute(httpServletRequest); - } else if (helper.isErrorDispatcherType(servletRequest)) { + servletPath = adapter.getIncludeServletPathAttribute(httpServletRequest); + pathInfo = adapter.getIncludePathInfoAttribute(httpServletRequest); + } else if (adapter.isErrorDispatcherType(servletRequest)) { spanType = RequestDispatcherSpanType.ERROR; - servletPath = helper.getServletPath(httpServletRequest); + servletPath = adapter.getServletPath(httpServletRequest); } if (spanType != null && (areNotEqual(servletPathTL.get(), servletPath) || areNotEqual(pathInfoTL.get(), pathInfo))) { @@ -161,12 +154,12 @@ public static Object onS return ret; } - public static void onExitServlet(REQUEST servletRequest, - RESPONSE servletResponse, - @Nullable Object transactionOrScopeOrSpan, - @Nullable Throwable t, - Object thiz, - ServletHelper helper) { + public static void onExitServlet(ServletRequest servletRequest, + ServletResponse servletResponse, + @Nullable Object transactionOrScopeOrSpan, + @Nullable Throwable t, + Object thiz, + ServletApiAdapter adapter) { ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); if (tracer == null) { return; @@ -186,39 +179,38 @@ public static void onExi if (scope != null) { scope.close(); } - if (helper.isInstanceOfHttpServlet(thiz) && helper.isHttpServletRequest(servletRequest)) { + HttpServletRequest httpServletRequest = adapter.asHttpServletRequest(servletRequest); + HttpServletResponse httpServletResponse = adapter.asHttpServletResponse(servletResponse); + if (adapter.isInstanceOfHttpServlet(thiz) && httpServletRequest != null) { Transaction currentTransaction = tracer.currentTransaction(); if (currentTransaction != null) { - final HTTPREQUEST httpServletRequest = (HTTPREQUEST) servletRequest; - TransactionNameUtils.setTransactionNameByServletClass(helper.getMethod(httpServletRequest), thiz.getClass(), currentTransaction.getAndOverrideName(PRIO_LOW_LEVEL_FRAMEWORK)); - final Principal userPrincipal = helper.getUserPrincipal(httpServletRequest); + TransactionNameUtils.setTransactionNameByServletClass(adapter.getMethod(httpServletRequest), thiz.getClass(), currentTransaction.getAndOverrideName(PRIO_LOW_LEVEL_FRAMEWORK)); + final Principal userPrincipal = adapter.getUserPrincipal(httpServletRequest); ServletTransactionHelper.setUsernameIfUnset(userPrincipal != null ? userPrincipal.getName() : null, currentTransaction.getContext()); } } if (transaction != null && - helper.isHttpServletRequest(servletRequest) && - helper.isHttpServletResponse(servletResponse)) { + httpServletRequest != null && + httpServletResponse != null) { - final HTTPREQUEST httpServletRequest = (HTTPREQUEST) servletRequest; - if (helper.getHttpAttribute(httpServletRequest, ServletTransactionHelper.ASYNC_ATTRIBUTE) != null) { + if (adapter.getHttpAttribute(httpServletRequest, ServletTransactionHelper.ASYNC_ATTRIBUTE) != null) { // HttpServletRequest.startAsync was invoked on this httpServletRequest. // The transaction should be handled from now on by the other thread committing the response transaction.deactivate(); } else { // this is not an async httpServletRequest, so we can end the transaction immediately - final HTTPRESPONSE response = (HTTPRESPONSE) servletResponse; if (transaction.isSampled() && tracer.getConfig(CoreConfiguration.class).isCaptureHeaders()) { final Response resp = transaction.getContext().getResponse(); - for (String headerName : helper.getHeaderNames(response)) { - resp.addHeader(headerName, helper.getHeaders(response, headerName)); + for (String headerName : adapter.getHeaderNames(httpServletResponse)) { + resp.addHeader(headerName, adapter.getHeaders(httpServletResponse, headerName)); } } // httpServletRequest.getParameterMap() may allocate a new map, depending on the servlet container implementation // so only call this method if necessary - final String contentTypeHeader = helper.getHeader(httpServletRequest, "Content-Type"); + final String contentTypeHeader = adapter.getHeader(httpServletRequest, "Content-Type"); final Map parameterMap; - if (transaction.isSampled() && servletTransactionHelper.captureParameters(helper.getMethod(httpServletRequest), contentTypeHeader)) { - parameterMap = helper.getParameterMap(httpServletRequest); + if (transaction.isSampled() && servletTransactionHelper.captureParameters(adapter.getMethod(httpServletRequest), contentTypeHeader)) { + parameterMap = adapter.getParameterMap(httpServletRequest); } else { parameterMap = null; } @@ -229,7 +221,7 @@ public static void onExi final int size = requestExceptionAttributes.size(); for (int i = 0; i < size; i++) { String attributeName = requestExceptionAttributes.get(i); - Object throwable = helper.getHttpAttribute(httpServletRequest, attributeName); + Object throwable = adapter.getHttpAttribute(httpServletRequest, attributeName); if (throwable instanceof Throwable) { t2 = (Throwable) throwable; if (!attributeName.equals("javax.servlet.error.exception") && !attributeName.equals("jakarta.servlet.error.exception")) { @@ -240,9 +232,9 @@ public static void onExi } } - servletTransactionHelper.onAfter(transaction, t == null ? t2 : t, helper.isCommitted(response), helper.getStatus(response), - overrideStatusCodeOnThrowable, helper.getMethod(httpServletRequest), parameterMap, helper.getServletPath(httpServletRequest), - helper.getPathInfo(httpServletRequest), contentTypeHeader, true + servletTransactionHelper.onAfter(transaction, t == null ? t2 : t, adapter.isCommitted(httpServletResponse), adapter.getStatus(httpServletResponse), + overrideStatusCodeOnThrowable, adapter.getMethod(httpServletRequest), parameterMap, adapter.getServletPath(httpServletRequest), + adapter.getPathInfo(httpServletRequest), contentTypeHeader, true ); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletGlobalState.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletGlobalState.java deleted file mode 100644 index 36bae0e033..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletGlobalState.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * 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.servlet; - -import co.elastic.apm.agent.sdk.state.GlobalState; -import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; -import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; - -@GlobalState -public class ServletGlobalState { - - public static final WeakMap nameInitialized = WeakConcurrent.buildMap(); - - // visible for testing as clearing cache is required between tests execution - static void clearServiceNameCache() { - nameInitialized.clear(); - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java new file mode 100644 index 0000000000..f7bccda519 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java @@ -0,0 +1,101 @@ +/* + * 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.servlet; + +import co.elastic.apm.agent.configuration.ServiceInfo; +import co.elastic.apm.agent.impl.Tracer; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.sdk.state.GlobalState; +import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; +import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; + +import javax.annotation.Nullable; +import java.io.InputStream; +import java.util.jar.JarFile; +import java.util.jar.Manifest; + +@GlobalState +public class ServletServiceNameHelper { + + public static final WeakMap nameInitialized = WeakConcurrent.buildMap(); + private static final Logger logger = LoggerFactory.getLogger(ServletServiceNameHelper.class); + + // this makes sure service name discovery also works when attaching at runtime + public static void determineServiceName( + ServletApiAdapter helper, + HttpServletRequest request, + Tracer tracer) { + + ServletContext servletContext = helper.getServletContext(request); + if (servletContext == null) { + return; + } + ClassLoader servletContextClassLoader = helper.getClassLoader(servletContext); + if (servletContextClassLoader == null || nameInitialized.putIfAbsent(servletContextClassLoader, Boolean.TRUE) != null) { + return; + } + String servletContextName = helper.getServletContextName(servletContext); + String contextPath = helper.getContextPath(servletContext); + + if (logger.isDebugEnabled()) { + logger.debug("Inferring service name for class loader [{}] based on servlet context path `{}` and request context path `{}`", + servletContextClassLoader, + servletContextName, + contextPath + ); + } + + ServiceInfo fromWarManifest = ServiceInfo.fromManifest(getManifest(helper, servletContext)); + ServiceInfo fromContextName = ServiceInfo.empty(); + if (!"application".equals(servletContextName) && !"".equals(servletContextName) && !"/".equals(servletContextName)) { + // payara returns an empty string as opposed to null + // spring applications which did not set spring.application.name have application as the default + // jetty returns context path when no display name is set, which could be the root context of "/" + // this is a worse default than the one we would otherwise choose + fromContextName = ServiceInfo.of(servletContextName); + } + ServiceInfo fromContextPath = ServiceInfo.empty(); + if (contextPath != null && !contextPath.isEmpty()) { + // remove leading slash + fromContextPath = ServiceInfo.of(contextPath.substring(1)); + } + tracer.overrideServiceInfoForClassLoader(servletContextClassLoader, fromWarManifest.withFallback(fromContextName).withFallback(fromContextPath)); + } + + @Nullable + public static Manifest getManifest( + ServletApiAdapter helper, + ServletContext servletContext) { + + try (InputStream manifestStream = helper.getResourceAsStream(servletContext, "/" + JarFile.MANIFEST_NAME)) { + if (manifestStream == null) { + return null; + } + return new Manifest(manifestStream); + } catch (Exception e) { + return null; + } + } + + // visible for testing as clearing cache is required between tests execution + static void clearServiceNameCache() { + nameInitialized.clear(); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index c7af9e0554..ef41ab2602 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -20,7 +20,6 @@ import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.GlobalTracer; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.context.Response; import co.elastic.apm.agent.impl.context.TransactionContext; @@ -28,9 +27,9 @@ import co.elastic.apm.agent.impl.context.web.WebConfiguration; import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.agent.matcher.WildcardMatcher; -import co.elastic.apm.agent.util.TransactionNameUtils; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.util.TransactionNameUtils; import javax.annotation.Nullable; import java.util.Arrays; @@ -41,7 +40,6 @@ import static co.elastic.apm.agent.configuration.CoreConfiguration.EventType.OFF; import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_DEFAULT; import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_LOW_LEVEL_FRAMEWORK; -import static co.elastic.apm.agent.servlet.ServletGlobalState.nameInitialized; public class ServletTransactionHelper { @@ -57,41 +55,48 @@ public class ServletTransactionHelper { private final Set METHODS_WITH_BODY = new HashSet<>(Arrays.asList("POST", "PUT", "PATCH", "DELETE")); private final CoreConfiguration coreConfiguration; private final WebConfiguration webConfiguration; + private final ElasticApmTracer tracer; public ServletTransactionHelper(ElasticApmTracer tracer) { this.coreConfiguration = tracer.getConfig(CoreConfiguration.class); this.webConfiguration = tracer.getConfig(WebConfiguration.class); + this.tracer = tracer; } - public static void determineServiceName(@Nullable String servletContextName, @Nullable ClassLoader servletContextClassLoader, @Nullable String contextPath) { - if (servletContextClassLoader == null || nameInitialized.putIfAbsent(servletContextClassLoader, Boolean.TRUE) != null) { - return; + @Nullable + public Transaction createAndActivateTransaction( + ServletApiAdapter adapter, + HttpServletRequest request) { + // only create a transaction if there is not already one + if (tracer.currentTransaction() != null) { + return null; } - - if (logger.isDebugEnabled()) { - logger.debug("Inferring service name for class loader [{}] based on servlet context path `{}` and request context path `{}`", - servletContextClassLoader, - servletContextName, - contextPath - ); + if (isExcluded(adapter.getHeader(request, "User-Agent"), adapter.getRequestURI(request))) { + return null; } + ClassLoader cl = adapter.getClassLoader(adapter.getServletContext(request)); + Transaction transaction = tracer.startChildTransaction(request, adapter.getRequestHeaderGetter(), cl); + if (transaction != null) { + transaction.activate(); + } + return transaction; + } + + public boolean isExcluded(@Nullable String userAgent, String requestUri) { + final WildcardMatcher excludeUrlMatcher = WildcardMatcher.anyMatch(webConfiguration.getIgnoreUrls(), requestUri); - @Nullable - String serviceName = servletContextName; - if ("application".equals(serviceName) || "".equals(serviceName) || "/".equals(serviceName)) { - // payara returns an empty string as opposed to null - // spring applications which did not set spring.application.name have application as the default - // jetty returns context path when no display name is set, which could be the root context of "/" - // this is a worse default than the one we would otherwise choose - serviceName = null; + if (excludeUrlMatcher != null && logger.isDebugEnabled()) { + logger.debug("Not tracing this request as the URL {} is ignored by the matcher {}", requestUri, excludeUrlMatcher); } - if (serviceName == null && contextPath != null && !contextPath.isEmpty()) { - // remove leading slash - serviceName = contextPath.substring(1); + final WildcardMatcher excludeAgentMatcher = userAgent != null ? WildcardMatcher.anyMatch(webConfiguration.getIgnoreUserAgents(), userAgent) : null; + if (excludeAgentMatcher != null) { + logger.debug("Not tracing this request as the User-Agent {} is ignored by the matcher {}", userAgent, excludeAgentMatcher); } - if (serviceName != null) { - GlobalTracer.get().overrideServiceInfoForClassLoader(servletContextClassLoader, serviceName); + boolean isExcluded = excludeUrlMatcher != null || excludeAgentMatcher != null; + if (!isExcluded && logger.isTraceEnabled()) { + logger.trace("No matcher found for excluding this request with URL: {}, and User-Agent: {}", requestUri, userAgent); } + return isExcluded; } public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/CommonServletRequestHeaderGetter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AbstractServletRequestHeaderGetter.java similarity index 79% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/CommonServletRequestHeaderGetter.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AbstractServletRequestHeaderGetter.java index 16bacede9f..73beb5cb7e 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/CommonServletRequestHeaderGetter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AbstractServletRequestHeaderGetter.java @@ -20,17 +20,9 @@ import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; -import javax.annotation.Nullable; import java.util.Enumeration; -public abstract class CommonServletRequestHeaderGetter implements TextHeaderGetter { - @Nullable - @Override - public String getFirstHeader(String headerName, T carrier) { - return getHeader(headerName, carrier); - } - - abstract String getHeader(String headerName, T carrier); +public abstract class AbstractServletRequestHeaderGetter implements TextHeaderGetter { @Override public void forEach(String headerName, T carrier, S state, HeaderConsumer consumer) { diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletRequestHeaderGetter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletRequestHeaderGetter.java index 0afa747ff9..69404eeea7 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletRequestHeaderGetter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletRequestHeaderGetter.java @@ -18,19 +18,21 @@ */ package co.elastic.apm.agent.servlet.helper; +import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import jakarta.servlet.http.HttpServletRequest; + import java.util.Enumeration; -public class JakartaServletRequestHeaderGetter extends CommonServletRequestHeaderGetter { +public class JakartaServletRequestHeaderGetter extends AbstractServletRequestHeaderGetter { private static final JakartaServletRequestHeaderGetter INSTANCE = new JakartaServletRequestHeaderGetter(); - static CommonServletRequestHeaderGetter getInstance() { + public static TextHeaderGetter getInstance() { return INSTANCE; } @Override - String getHeader(String headerName, HttpServletRequest carrier) { + public String getFirstHeader(String headerName, HttpServletRequest carrier) { return carrier.getHeader(headerName); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletTransactionCreationHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletTransactionCreationHelper.java deleted file mode 100644 index ec8781424d..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaServletTransactionCreationHelper.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * 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.servlet.helper; - -import co.elastic.apm.agent.impl.ElasticApmTracer; - -import jakarta.servlet.ServletContext; -import jakarta.servlet.http.HttpServletRequest; - -public class JakartaServletTransactionCreationHelper extends ServletTransactionCreationHelper { - public JakartaServletTransactionCreationHelper(ElasticApmTracer tracer) { - super(tracer); - } - - @Override - protected String getServletPath(HttpServletRequest httpServletRequest) { - return httpServletRequest.getServletPath(); - } - - @Override - protected String getPathInfo(HttpServletRequest httpServletRequest) { - return httpServletRequest.getPathInfo(); - } - - @Override - protected String getHeader(HttpServletRequest httpServletRequest, String headerName) { - return httpServletRequest.getHeader(headerName); - } - - @Override - protected ServletContext getServletContext(HttpServletRequest httpServletRequest) { - return httpServletRequest.getServletContext(); - } - - @Override - protected ClassLoader getClassLoader(ServletContext servletContext) { - return servletContext.getClassLoader(); - } - - @Override - protected CommonServletRequestHeaderGetter getRequestHeaderGetter() { - return JakartaServletRequestHeaderGetter.getInstance(); - } - - @Override - protected String getContextPath(HttpServletRequest httpServletRequest) { - return httpServletRequest.getContextPath(); - } - - @Override - protected String getRequestURI(HttpServletRequest httpServletRequest) { - return httpServletRequest.getRequestURI(); - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletRequestHeaderGetter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletRequestHeaderGetter.java index cb91aadadb..145e27a8d1 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletRequestHeaderGetter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletRequestHeaderGetter.java @@ -18,19 +18,21 @@ */ package co.elastic.apm.agent.servlet.helper; +import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; + import javax.servlet.http.HttpServletRequest; import java.util.Enumeration; -public class JavaxServletRequestHeaderGetter extends CommonServletRequestHeaderGetter { +public class JavaxServletRequestHeaderGetter extends AbstractServletRequestHeaderGetter { private static final JavaxServletRequestHeaderGetter INSTANCE = new JavaxServletRequestHeaderGetter(); - static CommonServletRequestHeaderGetter getInstance() { + public static TextHeaderGetter getInstance() { return INSTANCE; } @Override - String getHeader(String headerName, HttpServletRequest carrier) { + public String getFirstHeader(String headerName, HttpServletRequest carrier) { return carrier.getHeader(headerName); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletTransactionCreationHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletTransactionCreationHelper.java deleted file mode 100644 index b509778fbe..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxServletTransactionCreationHelper.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * 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.servlet.helper; - -import co.elastic.apm.agent.impl.ElasticApmTracer; - -import javax.servlet.ServletContext; -import javax.servlet.http.HttpServletRequest; - -public class JavaxServletTransactionCreationHelper extends ServletTransactionCreationHelper { - - public JavaxServletTransactionCreationHelper(ElasticApmTracer tracer) { - super(tracer); - } - - @Override - protected String getServletPath(HttpServletRequest httpServletRequest) { - return httpServletRequest.getServletPath(); - } - - @Override - protected String getPathInfo(HttpServletRequest httpServletRequest) { - return httpServletRequest.getPathInfo(); - } - - @Override - protected String getHeader(HttpServletRequest httpServletRequest, String headerName) { - return httpServletRequest.getHeader(headerName); - } - - @Override - protected ServletContext getServletContext(HttpServletRequest httpServletRequest) { - return httpServletRequest.getServletContext(); - } - - @Override - protected ClassLoader getClassLoader(ServletContext servletContext) { - return servletContext.getClassLoader(); - } - - @Override - protected CommonServletRequestHeaderGetter getRequestHeaderGetter() { - return JavaxServletRequestHeaderGetter.getInstance(); - } - - @Override - protected String getContextPath(HttpServletRequest httpServletRequest) { - return httpServletRequest.getContextPath(); - } - - @Override - protected String getRequestURI(HttpServletRequest httpServletRequest) { - return httpServletRequest.getRequestURI(); - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java deleted file mode 100644 index 4b472b65be..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * 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.servlet.helper; - -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.Tracer; -import co.elastic.apm.agent.impl.context.web.WebConfiguration; -import co.elastic.apm.agent.impl.transaction.Transaction; -import co.elastic.apm.agent.matcher.WildcardMatcher; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; - -import javax.annotation.Nullable; - -public abstract class ServletTransactionCreationHelper { - - private static final Logger logger = LoggerFactory.getLogger(ServletTransactionCreationHelper.class); - - private final Tracer tracer; - private final WebConfiguration webConfiguration; - - public ServletTransactionCreationHelper(ElasticApmTracer tracer) { - this.tracer = tracer; - webConfiguration = tracer.getConfig(WebConfiguration.class); - } - - @Nullable - public Transaction createAndActivateTransaction(HTTPREQUEST request) { - // only create a transaction if there is not already one - if (tracer.currentTransaction() != null) { - return null; - } - if (isExcluded(request)) { - return null; - } - ClassLoader cl = getClassloader(getServletContext(request)); - Transaction transaction = tracer.startChildTransaction(request, getRequestHeaderGetter(), cl); - if (transaction != null) { - transaction.activate(); - } - return transaction; - } - - protected abstract String getServletPath(HTTPREQUEST request); - - protected abstract String getPathInfo(HTTPREQUEST request); - - protected abstract String getHeader(HTTPREQUEST request, String headerName); - - protected abstract CONTEXT getServletContext(HTTPREQUEST request); - - protected abstract ClassLoader getClassLoader(CONTEXT servletContext); - - protected abstract CommonServletRequestHeaderGetter getRequestHeaderGetter(); - - protected abstract String getContextPath(HTTPREQUEST request); - - protected abstract String getRequestURI(HTTPREQUEST request); - - boolean isExcluded(HTTPREQUEST request) { - String userAgent = getHeader(request, "User-Agent"); - String requestUri = getRequestURI(request); - - final WildcardMatcher excludeUrlMatcher = WildcardMatcher.anyMatch(webConfiguration.getIgnoreUrls(), requestUri); - - if (excludeUrlMatcher != null && logger.isDebugEnabled()) { - logger.debug("Not tracing this request as the URL {} is ignored by the matcher {}", requestUri, excludeUrlMatcher); - } - final WildcardMatcher excludeAgentMatcher = userAgent != null ? WildcardMatcher.anyMatch(webConfiguration.getIgnoreUserAgents(), userAgent) : null; - if (excludeAgentMatcher != null) { - logger.debug("Not tracing this request as the User-Agent {} is ignored by the matcher {}", userAgent, excludeAgentMatcher); - } - boolean isExcluded = excludeUrlMatcher != null || excludeAgentMatcher != null; - if (!isExcluded && logger.isTraceEnabled()) { - logger.trace("No matcher found for excluding this request with URL: {}, and User-Agent: {}", requestUri, userAgent); - } - return isExcluded; - } - - @Nullable - public ClassLoader getClassloader(@Nullable CONTEXT servletContext) { - if (servletContext == null) { - return null; - } - - // getClassloader might throw UnsupportedOperationException - // see Section 4.4 of the Servlet 3.0 specification - ClassLoader classLoader = null; - try { - return getClassLoader(servletContext); - } catch (UnsupportedOperationException e) { - // silently ignored - return null; - } - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java index ba16a36b42..52b405e3a8 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java @@ -46,7 +46,7 @@ abstract class AbstractServletTest extends AbstractInstrumentationTest { void initServerAndClient() throws Exception { // because we reuse the same classloader with different servlet context names // we need to explicitly reset the name cache to make service name detection work as expected - ServletGlobalState.clearServiceNameCache(); + ServletServiceNameHelper.clearServiceNameCache(); // server is not reused between tests as handler is provided from subclass // another alternative diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java new file mode 100644 index 0000000000..0796285490 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java @@ -0,0 +1,117 @@ +/* + * 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.servlet; + +import co.elastic.apm.agent.MockReporter; +import co.elastic.apm.agent.MockTracer; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockServletContext; + +import java.io.InputStream; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.function.Supplier; +import java.util.jar.JarFile; + +import static org.assertj.core.api.Assertions.assertThat; + +class ServletServiceNameHelperTest { + + private final MockReporter reporter = new MockReporter(); + private final ElasticApmTracer tracer = MockTracer.createRealTracer(reporter); + + @BeforeEach + void setUp() { + } + + /** + * Tests a scenario of un-deploying a webapp and then re-deploying it on a Servlet container + */ + @Test + void testServiceNameConsistencyAcrossDifferentClassLoaders() { + + ClassLoader cl1 = new CustomManifestLoader(() -> null); + withThreadContextClassLoader(cl1, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + tracer.startRootTransaction(cl1).end(); + }); + + ClassLoader cl2 = new CustomManifestLoader(() -> null); + withThreadContextClassLoader(cl2, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + tracer.startRootTransaction(cl2).end(); + }); + + assertThat(reporter.getTransactions()).hasSize(2); + assertThat(reporter.getTransactions().stream() + .map(t -> t.getTraceContext().getServiceName()) + .distinct()) + .containsExactly("test-context"); + } + + @Test + void testServiceNameFromManifest() { + ClassLoader cl1 = new CustomManifestLoader(() -> getClass().getResourceAsStream("/TEST-MANIFEST.MF")); + withThreadContextClassLoader(cl1, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + tracer.startRootTransaction(cl1).end(); + }); + assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("service-name-from-manifest"); + assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("1.42.0"); + } + + @NotNull + private MockHttpServletRequest createRequest() { + MockServletContext servletContext = new MockServletContext(); + servletContext.setContextPath("test-context-path"); + servletContext.setServletContextName("test-context"); + return new MockHttpServletRequest(servletContext); + } + + private void withThreadContextClassLoader(ClassLoader contextClassLoader, Runnable runnable) { + ClassLoader previous = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader(contextClassLoader); + runnable.run(); + } finally { + Thread.currentThread().setContextClassLoader(previous); + } + } + + private static class CustomManifestLoader extends URLClassLoader { + private final Supplier manifestSupplier; + + public CustomManifestLoader(Supplier manifestSupplier) { + super(new URL[0]); + this.manifestSupplier = manifestSupplier; + } + + @Override + public InputStream getResourceAsStream(String name) { + if ((JarFile.MANIFEST_NAME).equals(name)) { + return manifestSupplier.get(); + } + return super.getResourceAsStream(name); + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletTransactionHelperTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletTransactionHelperTest.java index 7de6a9047d..ed2c80eedf 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletTransactionHelperTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletTransactionHelperTest.java @@ -28,8 +28,6 @@ import org.junit.jupiter.api.Test; import javax.annotation.Nonnull; -import java.net.URL; -import java.net.URLClassLoader; import java.util.List; import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_LOW_LEVEL_FRAMEWORK; @@ -88,28 +86,6 @@ void testGroupUrlsOverridesServletName() { assertThat(transaction.getNameAsString()).isEqualTo("GET /foo/bar/*"); } - /** - * Tests a scenario of un-deploying a webapp and then re-deploying it on a Servlet container - */ - @Test - void testServiceNameConsistencyAcrossDifferentClassLoaders() { - final String testContext = "test-context"; - final String testContextPath = "test-context-path"; - - URLClassLoader cl1 = new URLClassLoader(new URL[0]); - ServletTransactionHelper.determineServiceName(testContext, cl1, testContextPath); - tracer.startRootTransaction(cl1).end(); - - URLClassLoader cl2 = new URLClassLoader(new URL[0]); - ServletTransactionHelper.determineServiceName(testContext, cl2, testContextPath); - tracer.startRootTransaction(cl2).end(); - - assertThat(reporter.getTransactions().stream() - .filter(transaction -> testContext.equals(transaction.getTraceContext().getServiceName())) - .count() - ).isEqualTo(2); - } - @Nonnull private String getTransactionName(String method, String path) { Transaction transaction = new Transaction(MockTracer.create()); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java new file mode 100644 index 0000000000..ad3d4631cb --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java @@ -0,0 +1,48 @@ +/* + * 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.servlet.helper; + +import co.elastic.apm.agent.servlet.JavaxServletApiAdapter; +import org.junit.jupiter.api.Test; + +import javax.servlet.ServletContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ServletApiAdapterTest { + + public JavaxServletApiAdapter adapter = JavaxServletApiAdapter.get(); + + @Test + void safeGetClassLoader() { + assertThat(adapter.getClassLoader(null)).isNull(); + + ServletContext servletContext = mock(ServletContext.class); + doThrow(UnsupportedOperationException.class).when(servletContext).getClassLoader(); + assertThat(adapter.getClassLoader(servletContext)).isNull(); + + servletContext = mock(ServletContext.class); + ClassLoader cl = mock(ClassLoader.class); + when(servletContext.getClassLoader()).thenReturn(cl); + assertThat(adapter.getClassLoader(servletContext)).isSameAs(cl); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperTest.java index 9818ff2b06..7d73fb7aae 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperTest.java @@ -21,33 +21,29 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.impl.context.web.WebConfiguration; import co.elastic.apm.agent.matcher.WildcardMatcher; +import co.elastic.apm.agent.servlet.ServletTransactionHelper; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -import org.springframework.mock.web.MockHttpServletRequest; import javax.annotation.Nullable; -import javax.servlet.ServletContext; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; class ServletTransactionCreationHelperTest extends AbstractInstrumentationTest { private WebConfiguration webConfig; - private ServletTransactionCreationHelper helper; + private ServletTransactionHelper helper; @BeforeEach void setUp() { webConfig = config.getConfig(WebConfiguration.class); - helper = new JavaxServletTransactionCreationHelper(tracer); + helper = new ServletTransactionHelper(tracer); } @ParameterizedTest @@ -74,10 +70,7 @@ void checkRequestPathIgnored(String path, String config, boolean expectIgnored) when(webConfig.getIgnoreUrls()) .thenReturn(parseWildcard(config)); - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setRequestURI(path); - - boolean isIgnored = helper.isExcluded(request); + boolean isIgnored = helper.isExcluded(null, path); assertThat(isIgnored) .describedAs("request with path '%s' %s be ignored", expectIgnored ? "should" : "should not", path) .isEqualTo(expectIgnored); @@ -95,11 +88,7 @@ void requestUserAgentIgnored(String userAgent, String ignoreExpr) { when(webConfig.getIgnoreUserAgents()) .thenReturn(parseWildcard(ignoreExpr)); - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setRequestURI("/request/path"); - request.addHeader("user-agent", userAgent); - - assertThat(helper.isExcluded(request)) + assertThat(helper.isExcluded(userAgent, "/request/path")) .describedAs("request with user-agent '%s' should be ignored", userAgent) .isTrue(); } @@ -113,17 +102,4 @@ private static List parseWildcard(@Nullable String expr) { .collect(Collectors.toList()); } - @Test - void safeGetClassLoader() { - assertThat(helper.getClassloader(null)).isNull(); - - ServletContext servletContext = mock(ServletContext.class); - doThrow(UnsupportedOperationException.class).when(servletContext).getClassLoader(); - assertThat(helper.getClassloader(servletContext)).isNull(); - - servletContext = mock(ServletContext.class); - ClassLoader cl = mock(ClassLoader.class); - when(servletContext.getClassLoader()).thenReturn(cl); - assertThat(helper.getClassloader(servletContext)).isSameAs(cl); - } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/resources/TEST-MANIFEST.MF b/apm-agent-plugins/apm-servlet-plugin/src/test/resources/TEST-MANIFEST.MF new file mode 100644 index 0000000000..1b1d705409 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/resources/TEST-MANIFEST.MF @@ -0,0 +1,2 @@ +Implementation-Title: service$name-from-manifest +Implementation-Version: 1.42.0 diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/SpringServiceNameInstrumentation.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/SpringServiceNameInstrumentation.java index 8ad7cad261..e0922c121d 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/SpringServiceNameInstrumentation.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/SpringServiceNameInstrumentation.java @@ -19,13 +19,14 @@ package co.elastic.apm.agent.springwebmvc; import co.elastic.apm.agent.bci.TracerAwareInstrumentation; +import co.elastic.apm.agent.configuration.ServiceInfo; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; import org.springframework.web.context.WebApplicationContext; import javax.servlet.ServletContext; @@ -109,7 +110,7 @@ public static void afterInitPropertySources(@Advice.This WebApplicationContext a } } - tracer.overrideServiceInfoForClassLoader(classLoader, appName); + tracer.overrideServiceInfoForClassLoader(classLoader, ServiceInfo.of(appName)); } } } 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 9b6a30c90d..746662a02b 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,6 +18,7 @@ */ package co.elastic.apm.servlet.tests; +import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.impl.transaction.Outcome; import co.elastic.apm.servlet.AbstractServletContainerIntegrationTest; @@ -91,7 +92,7 @@ private void executeTest(final AbstractServletContainerIntegrationTest container /** * Since we test custom transaction creation through the external plugin, the service name for this transaction cannot be - * captured through the {@link Tracer#overrideServiceInfoForClassLoader(java.lang.ClassLoader, java.lang.String)} mechanism. + * captured through the {@link Tracer#overrideServiceInfoForClassLoader(ClassLoader, ServiceInfo)} mechanism. */ @Nullable @Override From b771c1abc9bbdf02370154911684de63f71a461e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 31 Jan 2022 11:45:14 +0100 Subject: [PATCH 02/10] Fix servlet unit tests --- ...arkataServletApiAdvice.java => JakartaServletApiAdvice.java} | 2 +- .../java/co/elastic/apm/agent/servlet/ServletApiAdapter.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{JarkataServletApiAdvice.java => JakartaServletApiAdvice.java} (96%) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java similarity index 96% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java index 767726bbed..914dd6f5f9 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JarkataServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java @@ -24,7 +24,7 @@ import javax.annotation.Nullable; -public class JarkataServletApiAdvice extends ServletApiAdvice { +public class JakartaServletApiAdvice extends ServletApiAdvice { private static final JakartaServletApiAdapter helper = JakartaServletApiAdapter.get(); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java index 3974b5dfcb..d3d5f09188 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java @@ -20,6 +20,7 @@ import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; +import co.elastic.apm.agent.sdk.state.GlobalState; import javax.annotation.Nullable; import java.io.InputStream; @@ -28,6 +29,7 @@ import java.util.Enumeration; import java.util.Map; +@GlobalState public interface ServletApiAdapter { @Nullable From ba5e5ab7bf225f259f6da991a28adadbd0c78258 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 31 Jan 2022 16:44:20 +0100 Subject: [PATCH 03/10] Instrument Filter::init and Servlet::init --- .../JakartaServletInstrumentationTest.java | 1 - .../InitServiceNameInstrumentation.java | 128 ++++++++++++++++++ .../servlet/JakartaServletApiAdvice.java | 7 +- .../agent/servlet/JavaxServletApiAdvice.java | 3 +- .../apm/agent/servlet/ServletApiAdvice.java | 47 ++++--- .../servlet/ServletServiceNameHelper.java | 23 ++-- .../servlet/ServletTransactionHelper.java | 13 +- .../agent/servlet/adapter/FilterAdapter.java | 25 ++++ .../JakartaServletApiAdapter.java | 34 +++-- .../{ => adapter}/JavaxServletApiAdapter.java | 34 +++-- .../agent/servlet/adapter/ServletAdapter.java | 29 ++++ .../servlet/adapter/ServletApiAdapter.java | 28 ++++ .../adapter/ServletContextAdapter.java | 37 +++++ .../ServletRequestAdapter.java} | 45 ++---- .../ServletRequestResponseAdapter.java | 26 ++++ .../adapter/ServletResponseAdapter.java | 39 ++++++ ...ic.apm.agent.sdk.ElasticApmInstrumentation | 2 + .../agent/servlet/CustomManifestLoader.java | 57 ++++++++ .../InitServiceNameInstrumentationTest.java | 88 ++++++++++++ .../servlet/ServletInstrumentationTest.java | 1 - .../servlet/ServletServiceNameHelperTest.java | 52 ++----- .../servlet/helper/ServletApiAdapterTest.java | 2 +- 22 files changed, 573 insertions(+), 148 deletions(-) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{ => adapter}/JakartaServletApiAdapter.java (87%) rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{ => adapter}/JavaxServletApiAdapter.java (87%) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java rename apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/{ServletApiAdapter.java => adapter/ServletRequestAdapter.java} (65%) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java diff --git a/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/JakartaServletInstrumentationTest.java b/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/JakartaServletInstrumentationTest.java index c4e1536c23..cf4eef1666 100644 --- a/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/JakartaServletInstrumentationTest.java +++ b/apm-agent-plugins/apm-servlet-jakarta-test/src/test/java/co/elastic/apm/agent/servlet/JakartaServletInstrumentationTest.java @@ -43,7 +43,6 @@ import java.io.IOException; import java.io.PrintWriter; -import java.util.Collections; import java.util.EnumSet; import static co.elastic.apm.agent.servlet.RequestDispatcherSpanType.FORWARD; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java new file mode 100644 index 0000000000..4d9ef40f39 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java @@ -0,0 +1,128 @@ +/* + * 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.servlet; + +import co.elastic.apm.agent.servlet.adapter.JakartaServletApiAdapter; +import co.elastic.apm.agent.servlet.adapter.JavaxServletApiAdapter; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.NamedElement; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +import javax.annotation.Nullable; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +/** + * Instruments + *
    + *
  • {@link javax.servlet.Filter#init(javax.servlet.FilterConfig)}
    • + *
  • {@link jakarta.servlet.Filter#init(jakarta.servlet.FilterConfig)}
    • + *
  • {@link javax.servlet.Servlet#init(javax.servlet.ServletConfig)}
    • + *
  • {@link jakarta.servlet.Servlet#init(jakarta.servlet.ServletConfig)}
    • + *
+ * + * Determines the service name based on the webapp's {@code META-INF/MANIFEST.MF} file early in the startup process. + * As this doesn't work with runtime attachment, the service name is also determined when the first request comes in. + */ +public abstract class InitServiceNameInstrumentation extends AbstractServletInstrumentation { + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameContains("Filter").or(nameContains("Servlet")); + } + + @Override + public ElementMatcher getTypeMatcher() { + return not(isInterface()).and(hasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.Servlet", "jakarta.servlet.Filter", "jakarta.servlet.Servlet"))); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("init") + .and(takesArguments(1)) + .and(takesArgument(0, nameEndsWith("Config"))); + } + + public static class JavaxInitServiceNameInstrumentation extends InitServiceNameInstrumentation { + + private static final JavaxServletApiAdapter adapter = JavaxServletApiAdapter.get(); + + @Override + public String rootClassNameThatClassloaderCanLoad() { + return "javax.servlet.AsyncContext"; + } + + public static class AdviceClass { + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void onEnter(@Advice.Argument(0) @Nullable Object config) { + if (config == null) { + return; + } + javax.servlet.ServletContext servletContext; + if (config instanceof javax.servlet.FilterConfig) { + servletContext = adapter.getServletContextFromFilterConfig((javax.servlet.FilterConfig) config); + } else if (config instanceof javax.servlet.ServletConfig) { + servletContext = adapter.getServletContextFromServletConfig((javax.servlet.ServletConfig) config); + } else { + return; + } + ServletServiceNameHelper.determineServiceName(adapter, servletContext, tracer); + } + } + } + + public static class JakartaInitServiceNameInstrumentation extends InitServiceNameInstrumentation { + + private static final JakartaServletApiAdapter adapter = JakartaServletApiAdapter.get(); + + @Override + public String rootClassNameThatClassloaderCanLoad() { + return "jakarta.servlet.AsyncContext"; + } + + public static class AdviceClass { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void onEnter(@Advice.Argument(0) @Nullable Object config) { + if (config == null) { + return; + } + jakarta.servlet.ServletContext servletContext; + if (config instanceof jakarta.servlet.FilterConfig) { + servletContext = adapter.getServletContextFromFilterConfig((jakarta.servlet.FilterConfig) config); + } else if (config instanceof jakarta.servlet.ServletConfig) { + servletContext = adapter.getServletContextFromServletConfig((jakarta.servlet.ServletConfig) config); + } else { + return; + } + ServletServiceNameHelper.determineServiceName(adapter, servletContext, tracer); + } + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java index 914dd6f5f9..88aa718ead 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdvice.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.servlet; +import co.elastic.apm.agent.servlet.adapter.JakartaServletApiAdapter; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import net.bytebuddy.asm.Advice; @@ -26,12 +27,12 @@ public class JakartaServletApiAdvice extends ServletApiAdvice { - private static final JakartaServletApiAdapter helper = JakartaServletApiAdapter.get(); + private static final JakartaServletApiAdapter adapter = JakartaServletApiAdapter.get(); @Nullable @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static Object onEnterServletService(@Advice.Argument(0) ServletRequest servletRequest) { - return onServletEnter(helper, servletRequest); + return onServletEnter(adapter, servletRequest); } @@ -41,6 +42,6 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl @Advice.Enter @Nullable Object transactionOrScopeOrSpan, @Advice.Thrown @Nullable Throwable t, @Advice.This Object thiz) { - onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, helper); + onExitServlet(adapter, servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java index 938b6fdac9..7841f11eeb 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdvice.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.servlet; +import co.elastic.apm.agent.servlet.adapter.JavaxServletApiAdapter; import net.bytebuddy.asm.Advice; import javax.annotation.Nullable; @@ -41,6 +42,6 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl @Advice.Enter @Nullable Object transactionOrScopeOrSpan, @Advice.Thrown @Nullable Throwable t, @Advice.This Object thiz) { - onExitServlet(servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz, adapter); + onExitServlet(adapter, servletRequest, servletResponse, transactionOrScopeOrSpan, t, thiz); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index 642482aee8..6e1acfeb75 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -6,9 +6,7 @@ * 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 @@ -31,6 +29,7 @@ import co.elastic.apm.agent.sdk.state.GlobalVariables; import co.elastic.apm.agent.sdk.weakconcurrent.DetachedThreadLocal; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; +import co.elastic.apm.agent.servlet.adapter.ServletApiAdapter; import co.elastic.apm.agent.util.TransactionNameUtils; import javax.annotation.Nullable; @@ -57,36 +56,40 @@ public abstract class ServletApiAdvice { private static final List requestExceptionAttributes = Arrays.asList("javax.servlet.error.exception", "jakarta.servlet.error.exception", "exception", "org.springframework.web.servlet.DispatcherServlet.EXCEPTION", "co.elastic.apm.exception"); @Nullable - public static Object onServletEnter( - ServletApiAdapter adapter, - ServletRequest servletRequest) { + public static Object onServletEnter( + ServletApiAdapter adapter, + Object servletRequest) { ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); if (tracer == null) { return null; } + + final HttpServletRequest httpServletRequest = adapter.asHttpServletRequest(servletRequest); + if (httpServletRequest == null) { + return null; + } AbstractSpan ret = null; // re-activate transactions for async requests - final Transaction transactionAttr = (Transaction) adapter.getAttribute(servletRequest, TRANSACTION_ATTRIBUTE); + final Transaction transactionAttr = (Transaction) adapter.getAttribute(httpServletRequest, TRANSACTION_ATTRIBUTE); if (tracer.currentTransaction() == null && transactionAttr != null) { return transactionAttr.activateInScope(); } - final HttpServletRequest httpServletRequest = adapter.asHttpServletRequest(servletRequest); - if (!tracer.isRunning() || httpServletRequest == null) { + if (!tracer.isRunning()) { return null; } CoreConfiguration coreConfig = tracer.getConfig(CoreConfiguration.class); - if (adapter.isRequestDispatcherType(servletRequest)) { + if (adapter.isRequestDispatcherType(httpServletRequest)) { if (Boolean.TRUE == excluded.get()) { return null; } - ServletServiceNameHelper.determineServiceName(adapter, httpServletRequest, tracer); + ServletServiceNameHelper.determineServiceName(adapter, adapter.getServletContext(httpServletRequest), tracer); - Transaction transaction = servletTransactionHelper.createAndActivateTransaction(adapter, httpServletRequest); + Transaction transaction = servletTransactionHelper.createAndActivateTransaction(adapter, adapter, httpServletRequest); if (transaction == null) { // if the httpServletRequest is excluded, avoid matching all exclude patterns again on each filter invocation @@ -113,21 +116,21 @@ public static parent = tracer.getActive(); if (parent != null) { Object servletPath = null; Object pathInfo = null; RequestDispatcherSpanType spanType = null; - if (adapter.isForwardDispatcherType(servletRequest)) { + if (adapter.isForwardDispatcherType(httpServletRequest)) { spanType = RequestDispatcherSpanType.FORWARD; servletPath = adapter.getServletPath(httpServletRequest); pathInfo = adapter.getPathInfo(httpServletRequest); - } else if (adapter.isIncludeDispatcherType(servletRequest)) { + } else if (adapter.isIncludeDispatcherType(httpServletRequest)) { spanType = RequestDispatcherSpanType.INCLUDE; servletPath = adapter.getIncludeServletPathAttribute(httpServletRequest); pathInfo = adapter.getIncludePathInfoAttribute(httpServletRequest); - } else if (adapter.isErrorDispatcherType(servletRequest)) { + } else if (adapter.isErrorDispatcherType(httpServletRequest)) { spanType = RequestDispatcherSpanType.ERROR; servletPath = adapter.getServletPath(httpServletRequest); } @@ -154,12 +157,14 @@ public static void onExitServlet(ServletRequest servletRequest, - ServletResponse servletResponse, - @Nullable Object transactionOrScopeOrSpan, - @Nullable Throwable t, - Object thiz, - ServletApiAdapter adapter) { + public static void onExitServlet( + ServletApiAdapter adapter, + Object servletRequest, + Object servletResponse, + @Nullable Object transactionOrScopeOrSpan, + @Nullable Throwable t, + Object thiz) { + ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); if (tracer == null) { return; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java index f7bccda519..5ddd2011ce 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletServiceNameHelper.java @@ -25,6 +25,7 @@ import co.elastic.apm.agent.sdk.state.GlobalState; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; +import co.elastic.apm.agent.servlet.adapter.ServletContextAdapter; import javax.annotation.Nullable; import java.io.InputStream; @@ -38,21 +39,19 @@ public class ServletServiceNameHelper { private static final Logger logger = LoggerFactory.getLogger(ServletServiceNameHelper.class); // this makes sure service name discovery also works when attaching at runtime - public static void determineServiceName( - ServletApiAdapter helper, - HttpServletRequest request, - Tracer tracer) { + public static void determineServiceName(ServletContextAdapter adapter, + @Nullable ServletContext servletContext, + Tracer tracer) { - ServletContext servletContext = helper.getServletContext(request); if (servletContext == null) { return; } - ClassLoader servletContextClassLoader = helper.getClassLoader(servletContext); + ClassLoader servletContextClassLoader = adapter.getClassLoader(servletContext); if (servletContextClassLoader == null || nameInitialized.putIfAbsent(servletContextClassLoader, Boolean.TRUE) != null) { return; } - String servletContextName = helper.getServletContextName(servletContext); - String contextPath = helper.getContextPath(servletContext); + String servletContextName = adapter.getServletContextName(servletContext); + String contextPath = adapter.getContextPath(servletContext); if (logger.isDebugEnabled()) { logger.debug("Inferring service name for class loader [{}] based on servlet context path `{}` and request context path `{}`", @@ -62,7 +61,7 @@ public static Manifest getManifest( - ServletApiAdapter helper, - ServletContext servletContext) { + private static Manifest getManifest(ServletContextAdapter adapter, ServletContext servletContext) { - try (InputStream manifestStream = helper.getResourceAsStream(servletContext, "/" + JarFile.MANIFEST_NAME)) { + try (InputStream manifestStream = adapter.getResourceAsStream(servletContext, "/" + JarFile.MANIFEST_NAME)) { if (manifestStream == null) { return null; } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index ef41ab2602..3b6d3da873 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -29,6 +29,8 @@ import co.elastic.apm.agent.matcher.WildcardMatcher; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.servlet.adapter.ServletContextAdapter; +import co.elastic.apm.agent.servlet.adapter.ServletRequestAdapter; import co.elastic.apm.agent.util.TransactionNameUtils; import javax.annotation.Nullable; @@ -64,18 +66,19 @@ public ServletTransactionHelper(ElasticApmTracer tracer) { } @Nullable - public Transaction createAndActivateTransaction( - ServletApiAdapter adapter, + public Transaction createAndActivateTransaction( + ServletRequestAdapter requestAdapter, + ServletContextAdapter contextAdapter, HttpServletRequest request) { // only create a transaction if there is not already one if (tracer.currentTransaction() != null) { return null; } - if (isExcluded(adapter.getHeader(request, "User-Agent"), adapter.getRequestURI(request))) { + if (isExcluded(requestAdapter.getHeader(request, "User-Agent"), requestAdapter.getRequestURI(request))) { return null; } - ClassLoader cl = adapter.getClassLoader(adapter.getServletContext(request)); - Transaction transaction = tracer.startChildTransaction(request, adapter.getRequestHeaderGetter(), cl); + ClassLoader cl = contextAdapter.getClassLoader(requestAdapter.getServletContext(request)); + Transaction transaction = tracer.startChildTransaction(request, requestAdapter.getRequestHeaderGetter(), cl); if (transaction != null) { transaction.activate(); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java new file mode 100644 index 0000000000..938e6bc683 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java @@ -0,0 +1,25 @@ +/* + * 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.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +@GlobalState +public interface FilterAdapter { + ServletContext getServletContextFromFilterConfig(FilterConfig filterConfig); +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java similarity index 87% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java index 7f05124fe1..976b5409ef 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JakartaServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java @@ -16,16 +16,16 @@ * specific language governing permissions and limitations * under the License. */ -package co.elastic.apm.agent.servlet; +package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import co.elastic.apm.agent.servlet.helper.JakartaServletRequestHeaderGetter; import jakarta.servlet.DispatcherType; +import jakarta.servlet.FilterConfig; import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletConfig; import jakarta.servlet.ServletContext; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; @@ -38,7 +38,7 @@ import java.util.Enumeration; import java.util.Map; -public class JakartaServletApiAdapter implements ServletApiAdapter { +public class JakartaServletApiAdapter implements ServletApiAdapter { public static final JakartaServletApiAdapter INSTANCE = new JakartaServletApiAdapter(); @@ -51,7 +51,7 @@ public static JakartaServletApiAdapter get() { @Nullable @Override - public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { + public HttpServletRequest asHttpServletRequest(Object servletRequest) { if (servletRequest instanceof HttpServletRequest) { return (HttpServletRequest) servletRequest; } @@ -60,7 +60,7 @@ public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { @Nullable @Override - public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse) { + public HttpServletResponse asHttpServletResponse(Object servletResponse) { if (servletResponse instanceof HttpServletResponse) { return (HttpServletResponse) servletResponse; } @@ -68,27 +68,27 @@ public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse } @Override - public boolean isRequestDispatcherType(ServletRequest servletRequest) { + public boolean isRequestDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.REQUEST; } @Override - public boolean isAsyncDispatcherType(ServletRequest servletRequest) { + public boolean isAsyncDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.ASYNC; } @Override - public boolean isForwardDispatcherType(ServletRequest servletRequest) { + public boolean isForwardDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.FORWARD; } @Override - public boolean isIncludeDispatcherType(ServletRequest servletRequest) { + public boolean isIncludeDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.INCLUDE; } @Override - public boolean isErrorDispatcherType(ServletRequest servletRequest) { + public boolean isErrorDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.ERROR; } @@ -214,6 +214,11 @@ public boolean isInstanceOfHttpServlet(Object object) { return object instanceof HttpServlet; } + @Override + public ServletContext getServletContextFromServletConfig(ServletConfig filterConfig) { + return filterConfig.getServletContext(); + } + @Override public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { return httpServletRequest.getUserPrincipal(); @@ -221,7 +226,7 @@ public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { @Nullable @Override - public Object getAttribute(ServletRequest servletRequest, String attributeName) { + public Object getAttribute(HttpServletRequest servletRequest, String attributeName) { return servletRequest.getAttribute(attributeName); } @@ -270,4 +275,9 @@ public InputStream getResourceAsStream(ServletContext servletContext, String pat public TextHeaderGetter getRequestHeaderGetter() { return JakartaServletRequestHeaderGetter.getInstance(); } + + @Override + public ServletContext getServletContextFromFilterConfig(FilterConfig filterConfig) { + return filterConfig.getServletContext(); + } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java similarity index 87% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java index cf428cede3..9dbb19faaa 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/JavaxServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package co.elastic.apm.agent.servlet; +package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; @@ -24,10 +24,10 @@ import javax.annotation.Nullable; import javax.servlet.DispatcherType; +import javax.servlet.FilterConfig; import javax.servlet.RequestDispatcher; +import javax.servlet.ServletConfig; import javax.servlet.ServletContext; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -38,7 +38,7 @@ import java.util.Enumeration; import java.util.Map; -public class JavaxServletApiAdapter implements ServletApiAdapter { +public class JavaxServletApiAdapter implements ServletApiAdapter { private static final JavaxServletApiAdapter INSTANCE = new JavaxServletApiAdapter(); @@ -50,7 +50,7 @@ public static JavaxServletApiAdapter get() { } @Override - public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { + public HttpServletRequest asHttpServletRequest(Object servletRequest) { if (servletRequest instanceof HttpServletRequest) { return (HttpServletRequest) servletRequest; } @@ -59,7 +59,7 @@ public HttpServletRequest asHttpServletRequest(ServletRequest servletRequest) { @Nullable @Override - public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse) { + public HttpServletResponse asHttpServletResponse(Object servletResponse) { if (servletResponse instanceof HttpServletResponse) { return (HttpServletResponse) servletResponse; } @@ -67,27 +67,27 @@ public HttpServletResponse asHttpServletResponse(ServletResponse servletResponse } @Override - public boolean isRequestDispatcherType(ServletRequest servletRequest) { + public boolean isRequestDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.REQUEST; } @Override - public boolean isAsyncDispatcherType(ServletRequest servletRequest) { + public boolean isAsyncDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.ASYNC; } @Override - public boolean isForwardDispatcherType(ServletRequest servletRequest) { + public boolean isForwardDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.FORWARD; } @Override - public boolean isIncludeDispatcherType(ServletRequest servletRequest) { + public boolean isIncludeDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.INCLUDE; } @Override - public boolean isErrorDispatcherType(ServletRequest servletRequest) { + public boolean isErrorDispatcherType(HttpServletRequest servletRequest) { return servletRequest.getDispatcherType() == DispatcherType.ERROR; } @@ -213,6 +213,11 @@ public boolean isInstanceOfHttpServlet(Object object) { return object instanceof HttpServlet; } + @Override + public ServletContext getServletContextFromServletConfig(ServletConfig filterConfig) { + return filterConfig.getServletContext(); + } + @Override public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { return httpServletRequest.getUserPrincipal(); @@ -220,7 +225,7 @@ public Principal getUserPrincipal(HttpServletRequest httpServletRequest) { @Nullable @Override - public Object getAttribute(ServletRequest servletRequest, String attributeName) { + public Object getAttribute(HttpServletRequest servletRequest, String attributeName) { return servletRequest.getAttribute(attributeName); } @@ -270,4 +275,9 @@ public InputStream getResourceAsStream(ServletContext servletContext, String pat public TextHeaderGetter getRequestHeaderGetter() { return JavaxServletRequestHeaderGetter.getInstance(); } + + @Override + public ServletContext getServletContextFromFilterConfig(FilterConfig filterConfig) { + return filterConfig.getServletContext(); + } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java new file mode 100644 index 0000000000..d3f1b45e38 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java @@ -0,0 +1,29 @@ +/* + * 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.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +@GlobalState +public interface ServletAdapter { + + boolean isInstanceOfHttpServlet(Object object); + + ServletContext getServletContextFromServletConfig(ServletConfig filterConfig); + +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java new file mode 100644 index 0000000000..2e951b5d4a --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java @@ -0,0 +1,28 @@ +/* + * 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.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +@GlobalState +public interface ServletApiAdapter extends + ServletRequestResponseAdapter, + ServletContextAdapter, + ServletAdapter, + FilterAdapter { + +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java new file mode 100644 index 0000000000..024a64ec5c --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java @@ -0,0 +1,37 @@ +/* + * 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.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +import javax.annotation.Nullable; +import java.io.InputStream; + +@GlobalState +public interface ServletContextAdapter { + @Nullable + ClassLoader getClassLoader(@Nullable ServletContext servletContext); + + String getServletContextName(ServletContext servletContext); + + @Nullable + String getContextPath(ServletContext servletContext); + + @Nullable + InputStream getResourceAsStream(ServletContext servletContext, String path); +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestAdapter.java similarity index 65% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestAdapter.java index d3d5f09188..2163d75018 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestAdapter.java @@ -16,49 +16,35 @@ * specific language governing permissions and limitations * under the License. */ -package co.elastic.apm.agent.servlet; +package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import co.elastic.apm.agent.sdk.state.GlobalState; import javax.annotation.Nullable; -import java.io.InputStream; import java.security.Principal; -import java.util.Collection; import java.util.Enumeration; import java.util.Map; @GlobalState -public interface ServletApiAdapter { - - @Nullable - HttpServletRequest asHttpServletRequest(ServletRequest servletRequest); - +public interface ServletRequestAdapter { @Nullable - HttpServletResponse asHttpServletResponse(ServletResponse servletResponse); + HttpServletRequest asHttpServletRequest(Object servletRequest); - boolean isRequestDispatcherType(ServletRequest servletRequest); + boolean isRequestDispatcherType(HttpServletRequest servletRequest); - boolean isAsyncDispatcherType(ServletRequest servletRequest); + boolean isAsyncDispatcherType(HttpServletRequest servletRequest); - boolean isForwardDispatcherType(ServletRequest servletRequest); + boolean isForwardDispatcherType(HttpServletRequest servletRequest); - boolean isIncludeDispatcherType(ServletRequest servletRequest); + boolean isIncludeDispatcherType(HttpServletRequest servletRequest); - boolean isErrorDispatcherType(ServletRequest servletRequest); + boolean isErrorDispatcherType(HttpServletRequest servletRequest); @Nullable ServletContext getServletContext(HttpServletRequest servletRequest); - @Nullable - ClassLoader getClassLoader(@Nullable ServletContext servletContext); - - String getServletContextName(ServletContext servletContext); - - @Nullable - String getContextPath(ServletContext servletContext); - void handleCookies(Request request, HttpServletRequest httpServletRequest); @Nullable @@ -95,29 +81,16 @@ public interface ServletApiAdapter getHeaderNames(HttpServletResponse httpServletResponse); - - Collection getHeaders(HttpServletResponse httpServletResponse, String headerName); - Map getParameterMap(HttpServletRequest httpServletRequest); - boolean isCommitted(HttpServletResponse httpServletResponse); - - int getStatus(HttpServletResponse httpServletResponse); - - @Nullable - InputStream getResourceAsStream(ServletContext servletContext, String path); - TextHeaderGetter getRequestHeaderGetter(); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java new file mode 100644 index 0000000000..8ae65e1aed --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.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.agent.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +@GlobalState +public interface ServletRequestResponseAdapter extends + ServletRequestAdapter, + ServletResponseAdapter { +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java new file mode 100644 index 0000000000..af52556a0c --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java @@ -0,0 +1,39 @@ +/* + * 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.servlet.adapter; + +import co.elastic.apm.agent.sdk.state.GlobalState; + +import javax.annotation.Nullable; +import java.util.Collection; + +@GlobalState +public interface ServletResponseAdapter { + + @Nullable + HttpServletResponse asHttpServletResponse(Object servletResponse); + + Collection getHeaderNames(HttpServletResponse httpServletResponse); + + Collection getHeaders(HttpServletResponse httpServletResponse, String headerName); + + boolean isCommitted(HttpServletResponse httpServletResponse); + + int getStatus(HttpServletResponse httpServletResponse); + +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation index 92345979a1..cb4fab934c 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation @@ -14,3 +14,5 @@ co.elastic.apm.agent.servlet.JakartaAsyncInstrumentation$JakartaStartAsyncInstru co.elastic.apm.agent.servlet.JakartaAsyncInstrumentation$JakartaAsyncContextInstrumentation co.elastic.apm.agent.servlet.JavaxRequestStreamRecordingInstrumentation co.elastic.apm.agent.servlet.JakartaRequestStreamRecordingInstrumentation +co.elastic.apm.agent.servlet.InitServiceNameInstrumentation$JavaxInitServiceNameInstrumentation +co.elastic.apm.agent.servlet.InitServiceNameInstrumentation$JakartaInitServiceNameInstrumentation diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java new file mode 100644 index 0000000000..a384d0590b --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java @@ -0,0 +1,57 @@ +/* + * 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.servlet; + +import org.junit.function.ThrowingRunnable; + +import java.io.InputStream; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.function.Supplier; +import java.util.jar.JarFile; + +public class CustomManifestLoader extends URLClassLoader { + private final Supplier manifestSupplier; + + public CustomManifestLoader(Supplier manifestSupplier) { + super(new URL[0]); + this.manifestSupplier = manifestSupplier; + } + + public static void withThreadContextClassLoader(ClassLoader contextClassLoader, ThrowingRunnable runnable) { + ClassLoader previous = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader(contextClassLoader); + runnable.run(); + } catch (RuntimeException | Error e) { + throw e; + } catch (Throwable e) { + throw new RuntimeException(e); + } finally { + Thread.currentThread().setContextClassLoader(previous); + } + } + + @Override + public InputStream getResourceAsStream(String name) { + if ((JarFile.MANIFEST_NAME).equals(name)) { + return manifestSupplier.get(); + } + return super.getResourceAsStream(name); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java new file mode 100644 index 0000000000..c2e17731ba --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java @@ -0,0 +1,88 @@ +/* + * 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.servlet; + +import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.transaction.TraceContext; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockFilterConfig; +import org.springframework.mock.web.MockServletConfig; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.Servlet; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServlet; +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; + +class InitServiceNameInstrumentationTest extends AbstractInstrumentationTest { + + @Test + void testServletInit() { + Servlet servlet = new HttpServlet() { + }; + + CustomManifestLoader cl = new CustomManifestLoader(() -> getClass().getResourceAsStream("/TEST-MANIFEST.MF")); + CustomManifestLoader.withThreadContextClassLoader(cl, () -> { + servlet.init(new MockServletConfig()); + tracer.startRootTransaction(cl).end(); + }); + + assertServiceInfo(); + } + + @Test + void testFilterInit() { + Filter filter = new NoopFilter(); + + CustomManifestLoader cl = new CustomManifestLoader(() -> getClass().getResourceAsStream("/TEST-MANIFEST.MF")); + CustomManifestLoader.withThreadContextClassLoader(cl, () -> { + filter.init(new MockFilterConfig()); + tracer.startRootTransaction(cl).end(); + }); + + assertServiceInfo(); + } + + private void assertServiceInfo() { + TraceContext traceContext = reporter.getFirstTransaction().getTraceContext(); + assertThat(traceContext.getServiceName()).isEqualTo("service-name-from-manifest"); + assertThat(traceContext.getServiceVersion()).isEqualTo("1.42.0"); + } + + private static class NoopFilter implements Filter { + @Override + public void init(FilterConfig filterConfig) { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + chain.doFilter(request, response); + } + + @Override + public void destroy() { + + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java index 3b10cdb87e..fb398dcedf 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletInstrumentationTest.java @@ -43,7 +43,6 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintWriter; -import java.util.Collections; import java.util.EnumSet; import static co.elastic.apm.agent.servlet.RequestDispatcherSpanType.ERROR; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java index 0796285490..6a45b51323 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java @@ -6,9 +6,7 @@ * 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 @@ -21,17 +19,13 @@ import co.elastic.apm.agent.MockReporter; import co.elastic.apm.agent.MockTracer; import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.servlet.adapter.JavaxServletApiAdapter; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; -import java.io.InputStream; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.function.Supplier; -import java.util.jar.JarFile; +import javax.servlet.ServletContext; import static org.assertj.core.api.Assertions.assertThat; @@ -51,14 +45,14 @@ void setUp() { void testServiceNameConsistencyAcrossDifferentClassLoaders() { ClassLoader cl1 = new CustomManifestLoader(() -> null); - withThreadContextClassLoader(cl1, () -> { - ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + CustomManifestLoader.withThreadContextClassLoader(cl1, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createServletContext(), tracer); tracer.startRootTransaction(cl1).end(); }); ClassLoader cl2 = new CustomManifestLoader(() -> null); - withThreadContextClassLoader(cl2, () -> { - ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + CustomManifestLoader.withThreadContextClassLoader(cl2, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createServletContext(), tracer); tracer.startRootTransaction(cl2).end(); }); @@ -72,8 +66,8 @@ void testServiceNameConsistencyAcrossDifferentClassLoaders() { @Test void testServiceNameFromManifest() { ClassLoader cl1 = new CustomManifestLoader(() -> getClass().getResourceAsStream("/TEST-MANIFEST.MF")); - withThreadContextClassLoader(cl1, () -> { - ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createRequest(), tracer); + CustomManifestLoader.withThreadContextClassLoader(cl1, () -> { + ServletServiceNameHelper.determineServiceName(JavaxServletApiAdapter.get(), createServletContext(), tracer); tracer.startRootTransaction(cl1).end(); }); assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("service-name-from-manifest"); @@ -81,37 +75,11 @@ void testServiceNameFromManifest() { } @NotNull - private MockHttpServletRequest createRequest() { + private ServletContext createServletContext() { MockServletContext servletContext = new MockServletContext(); servletContext.setContextPath("test-context-path"); servletContext.setServletContextName("test-context"); - return new MockHttpServletRequest(servletContext); + return servletContext; } - private void withThreadContextClassLoader(ClassLoader contextClassLoader, Runnable runnable) { - ClassLoader previous = Thread.currentThread().getContextClassLoader(); - try { - Thread.currentThread().setContextClassLoader(contextClassLoader); - runnable.run(); - } finally { - Thread.currentThread().setContextClassLoader(previous); - } - } - - private static class CustomManifestLoader extends URLClassLoader { - private final Supplier manifestSupplier; - - public CustomManifestLoader(Supplier manifestSupplier) { - super(new URL[0]); - this.manifestSupplier = manifestSupplier; - } - - @Override - public InputStream getResourceAsStream(String name) { - if ((JarFile.MANIFEST_NAME).equals(name)) { - return manifestSupplier.get(); - } - return super.getResourceAsStream(name); - } - } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java index ad3d4631cb..90cb43d7d4 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletApiAdapterTest.java @@ -18,7 +18,7 @@ */ package co.elastic.apm.agent.servlet.helper; -import co.elastic.apm.agent.servlet.JavaxServletApiAdapter; +import co.elastic.apm.agent.servlet.adapter.JavaxServletApiAdapter; import org.junit.jupiter.api.Test; import javax.servlet.ServletContext; From bd146d438be1a664eb1c5214b604100472f26a30 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 31 Jan 2022 19:24:11 +0100 Subject: [PATCH 04/10] Implementation-Version detection integration test --- .../apm/agent/servlet/ServletApiAdvice.java | 2 ++ .../agent/servlet/adapter/FilterAdapter.java | 3 ++- .../agent/servlet/adapter/ServletAdapter.java | 3 ++- .../servlet/adapter/ServletApiAdapter.java | 2 ++ .../adapter/ServletContextAdapter.java | 3 ++- .../ServletRequestResponseAdapter.java | 3 ++- .../adapter/ServletResponseAdapter.java | 3 ++- .../agent/servlet/CustomManifestLoader.java | 3 ++- .../InitServiceNameInstrumentationTest.java | 3 ++- .../servlet/ServletServiceNameHelperTest.java | 2 ++ ...stractServletContainerIntegrationTest.java | 20 ++++++++++++------- integration-tests/simple-webapp/pom.xml | 13 ++++++++++++ 12 files changed, 46 insertions(+), 14 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index 6e1acfeb75..7af1b387be 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -6,7 +6,9 @@ * 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 diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java index 938e6bc683..e65ff90367 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/FilterAdapter.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.sdk.state.GlobalState; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java index d3f1b45e38..0f5a2ae074 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.sdk.state.GlobalState; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java index 2e951b5d4a..e8bdbe6d6f 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java @@ -6,7 +6,9 @@ * 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 diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java index 024a64ec5c..cebda44b24 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletContextAdapter.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.sdk.state.GlobalState; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java index 8ae65e1aed..0977f1b66d 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletRequestResponseAdapter.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.sdk.state.GlobalState; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java index af52556a0c..016409fa21 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletResponseAdapter.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet.adapter; import co.elastic.apm.agent.sdk.state.GlobalState; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java index a384d0590b..d8835c5926 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/CustomManifestLoader.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet; import org.junit.function.ThrowingRunnable; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java index c2e17731ba..e143bf806e 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java @@ -6,7 +6,9 @@ * 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 @@ -14,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package co.elastic.apm.agent.servlet; import co.elastic.apm.agent.AbstractInstrumentationTest; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java index 6a45b51323..0adc460651 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletServiceNameHelperTest.java @@ -6,7 +6,9 @@ * 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 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 d278d2f60a..2e9202a156 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 @@ -19,6 +19,8 @@ package co.elastic.apm.servlet; import co.elastic.apm.agent.MockReporter; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.testutils.TestContainersUtils; import co.elastic.apm.servlet.tests.TestApp; import com.fasterxml.jackson.databind.JsonNode; @@ -35,8 +37,6 @@ import org.mockserver.model.ClearType; import org.mockserver.model.HttpRequest; import org.mockserver.model.HttpResponse; -import co.elastic.apm.agent.sdk.logging.Logger; -import co.elastic.apm.agent.sdk.logging.LoggerFactory; import org.testcontainers.containers.Container; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; @@ -598,11 +598,17 @@ private void validateEventMetadata(String bodyAsString) { private void validateServiceName(JsonNode event) { String expectedServiceName = currentTestApp.getExpectedServiceName(); - if (expectedServiceName != null && event != null) { - JsonNode contextService = event.get("context").get("service"); - assertThat(contextService) - .withFailMessage("No service name set. Expected '%s'. Event was %s", expectedServiceName, event) - .isNotNull(); + if (event == null) { + return; + } + JsonNode contextService = event.get("context").get("service"); + assertThat(contextService) + .withFailMessage("No service context available.") + .isNotNull(); + assertThat(contextService.get("version").textValue()) + .describedAs("Event has no service version %s", event) + .isNotEmpty(); + if (expectedServiceName != null) { assertThat(contextService.get("name").textValue()) .describedAs("Event has unexpected service name %s", event) .isEqualTo(expectedServiceName); diff --git a/integration-tests/simple-webapp/pom.xml b/integration-tests/simple-webapp/pom.xml index 28e147f892..14eab3b98c 100644 --- a/integration-tests/simple-webapp/pom.xml +++ b/integration-tests/simple-webapp/pom.xml @@ -20,6 +20,19 @@ simple-webapp + + + org.apache.maven.plugins + maven-war-plugin + + + + ${project.version} + + + + + From fc0f4d908f5f629c61c86886f0c37ca2497f64c2 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 1 Feb 2022 10:57:15 +0100 Subject: [PATCH 05/10] Fix integration tests --- .../AbstractServletContainerIntegrationTest.java | 11 +++++++---- .../tests/AbstractJsfApplicationServerTestApp.java | 2 +- .../servlet/tests/AbstractServletApiTestApp.java | 4 ++-- .../servlet/tests/CdiApplicationServerTestApp.java | 3 ++- .../tests/CdiJakartaeeApplicationServerTestApp.java | 3 ++- .../tests/CdiJakartaeeServletContainerTestApp.java | 3 ++- .../servlet/tests/CdiServletContainerTestApp.java | 3 ++- .../apm/servlet/tests/ExternalPluginTestApp.java | 4 ++-- .../tests/JakartaeeJsfServletContainerTestApp.java | 3 ++- .../servlet/tests/JakartaeeServletApiTestApp.java | 2 +- .../servlet/tests/JsfServletContainerTestApp.java | 3 ++- .../apm/servlet/tests/ServletApiTestApp.java | 2 +- .../co/elastic/apm/servlet/tests/SoapTestApp.java | 3 ++- .../java/co/elastic/apm/servlet/tests/TestApp.java | 7 ++++++- integration-tests/jakartaee-simple-webapp/pom.xml | 13 +++++++++++++ integration-tests/simple-webapp/pom.xml | 2 +- 16 files changed, 48 insertions(+), 20 deletions(-) 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 2e9202a156..385f762609 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 @@ -598,21 +598,24 @@ private void validateEventMetadata(String bodyAsString) { private void validateServiceName(JsonNode event) { String expectedServiceName = currentTestApp.getExpectedServiceName(); - if (event == null) { + String expectedServiceVersion = currentTestApp.getExpectedServiceVersion(); + if (event == null || (expectedServiceName == null && expectedServiceVersion == null)) { return; } JsonNode contextService = event.get("context").get("service"); assertThat(contextService) .withFailMessage("No service context available.") .isNotNull(); - assertThat(contextService.get("version").textValue()) - .describedAs("Event has no service version %s", event) - .isNotEmpty(); if (expectedServiceName != null) { assertThat(contextService.get("name").textValue()) .describedAs("Event has unexpected service name %s", event) .isEqualTo(expectedServiceName); } + if (expectedServiceVersion != null) { + assertThat(contextService.get("version").textValue()) + .describedAs("Event has no service version %s", event) + .isEqualTo(expectedServiceVersion); + } } private void validateMetadataEvent(JsonNode metadata) { diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractJsfApplicationServerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractJsfApplicationServerTestApp.java index 8e67a83fb6..a69ebda294 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractJsfApplicationServerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractJsfApplicationServerTestApp.java @@ -38,7 +38,7 @@ public AbstractJsfApplicationServerTestApp(String servicePath, String deploymentContext, String statusEndpoint, @Nullable String expectedServiceName) { - super(modulePath, appFileName, deploymentContext, statusEndpoint, expectedServiceName); + super(modulePath, appFileName, deploymentContext, statusEndpoint, expectedServiceName, null); this.servicePath = servicePath; } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractServletApiTestApp.java index 03f2189052..660d496ec8 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/AbstractServletApiTestApp.java @@ -41,8 +41,8 @@ public abstract class AbstractServletApiTestApp extends TestApp { private final String servicePath; private final String spanName; - public AbstractServletApiTestApp(String modulePath, String appFileName, String statusEndpoint, String expectedServiceName, String servicePath, String spanName) { - super(modulePath, appFileName, servicePath, statusEndpoint, expectedServiceName); + public AbstractServletApiTestApp(String modulePath, String appFileName, String statusEndpoint, String expectedServiceName, String servicePath, String spanName, String expectedVersion) { + super(modulePath, appFileName, servicePath, statusEndpoint, expectedServiceName, expectedVersion); this.servicePath = servicePath; this.spanName = spanName; } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiApplicationServerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiApplicationServerTestApp.java index c5d55c6211..23496dde6e 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiApplicationServerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiApplicationServerTestApp.java @@ -33,7 +33,8 @@ public CdiApplicationServerTestApp() { "cdi-app.war", "cdi-app", "status.html", - "CDI App"); + "CDI App", + null); } @Override diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeApplicationServerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeApplicationServerTestApp.java index 8763f6e0b9..6da59b4206 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeApplicationServerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeApplicationServerTestApp.java @@ -33,7 +33,8 @@ public CdiJakartaeeApplicationServerTestApp() { "cdi-jakartaee-app.war", "cdi-jakartaee-app", "status.html", - "CDI Jakarta App"); + "CDI Jakarta App", + null); } @Override diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeServletContainerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeServletContainerTestApp.java index 023823753e..ce76984f1c 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeServletContainerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiJakartaeeServletContainerTestApp.java @@ -27,7 +27,8 @@ public CdiJakartaeeServletContainerTestApp() { "cdi-jakartaee-app.war", "cdi-jakartaee-app", "status.html", - "CDI Jakarta App"); + "CDI Jakarta App", + null); } @Override diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiServletContainerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiServletContainerTestApp.java index 28325350d2..9ef96b8674 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiServletContainerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiServletContainerTestApp.java @@ -27,7 +27,8 @@ public CdiServletContainerTestApp() { "cdi-app.war", "cdi-app", "status.html", - "CDI App"); + "CDI App", + null); } @Override 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 746662a02b..5c07b4cc4f 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 @@ -46,8 +46,8 @@ protected ExternalPluginTestApp(String testAppModuleName, String appWarFileName) appWarFileName + ".war", appWarFileName, "status.html", - appWarFileName - ); + appWarFileName, + null); this.appWarFileName = appWarFileName; } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeJsfServletContainerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeJsfServletContainerTestApp.java index ddf0239101..28b3c56cc3 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeJsfServletContainerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeJsfServletContainerTestApp.java @@ -26,7 +26,8 @@ public JakartaeeJsfServletContainerTestApp() { "jakartaee-jsf-http-get.war", "jakartaee-jsf-http-get", "status.html", - "jakartaee-jsf-http-get"); + "jakartaee-jsf-http-get", + null); } @Override diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeServletApiTestApp.java index ad493c9715..b44c3c51b1 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JakartaeeServletApiTestApp.java @@ -21,6 +21,6 @@ public class JakartaeeServletApiTestApp extends AbstractServletApiTestApp { public JakartaeeServletApiTestApp() { - super("../jakartaee-simple-webapp", "jakartaee-simple-webapp.war", "status.jsp", "Simple Web App", "jakartaee-simple-webapp", "JakartaTestApiServlet"); + super("../jakartaee-simple-webapp", "jakartaee-simple-webapp.war", "status.jsp", "Simple Web App", "jakartaee-simple-webapp", "JakartaTestApiServlet", "4.2.0"); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfServletContainerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfServletContainerTestApp.java index 50caa839a3..ba82819ce7 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfServletContainerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfServletContainerTestApp.java @@ -26,7 +26,8 @@ public JsfServletContainerTestApp() { "jsf-http-get.war", "jsf-http-get", "status.html", - "jsf-http-get"); + "jsf-http-get", + null); } @Override diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java index d55534f41f..d403c9edd5 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java @@ -21,7 +21,7 @@ public class ServletApiTestApp extends AbstractServletApiTestApp { public ServletApiTestApp() { - super("../simple-webapp", "simple-webapp.war", "status.jsp", "Simple Web App", "simple-webapp", "TestApiServlet"); + super("../simple-webapp", "simple-webapp.war", "status.jsp", "Simple Web App", "simple-webapp", "TestApiServlet", "4.2.0"); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/SoapTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/SoapTestApp.java index 3c82935138..d4ea5a5d8a 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/SoapTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/SoapTestApp.java @@ -34,7 +34,8 @@ public SoapTestApp() { "soap-test.war", "soap-test", "status.html", - "soap-test"); + "soap-test", + null); } @Override 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 7b96068b0d..ae8f288c17 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 @@ -33,10 +33,12 @@ public abstract class TestApp { @Nullable private final String expectedServiceName; private final String deploymentContext; + private final String expectedServiceVersion; - TestApp(String modulePath, String appFileName, String deploymentContext, String statusEndpoint, @Nullable String expectedServiceName) { + TestApp(String modulePath, String appFileName, String deploymentContext, String statusEndpoint, @Nullable String expectedServiceName, String expectedVersion) { this.modulePath = modulePath; this.appFileName = appFileName; + this.expectedServiceVersion = expectedVersion; this.statusEndpoint = String.format("/%s/%s", deploymentContext, statusEndpoint); this.deploymentContext = deploymentContext; this.expectedServiceName = expectedServiceName; @@ -92,4 +94,7 @@ public Map getAdditionalEnvVariables() { public abstract void test(AbstractServletContainerIntegrationTest test) throws Exception; + public String getExpectedServiceVersion() { + return expectedServiceVersion; + } } diff --git a/integration-tests/jakartaee-simple-webapp/pom.xml b/integration-tests/jakartaee-simple-webapp/pom.xml index 4bc9ed47ef..8e3717d39c 100644 --- a/integration-tests/jakartaee-simple-webapp/pom.xml +++ b/integration-tests/jakartaee-simple-webapp/pom.xml @@ -19,6 +19,19 @@ jakartaee-simple-webapp + + + org.apache.maven.plugins + maven-war-plugin + + + + 4.2.0 + + + + + diff --git a/integration-tests/simple-webapp/pom.xml b/integration-tests/simple-webapp/pom.xml index 14eab3b98c..ae0256215a 100644 --- a/integration-tests/simple-webapp/pom.xml +++ b/integration-tests/simple-webapp/pom.xml @@ -27,7 +27,7 @@ - ${project.version} + 4.2.0 From b95895155b168b5b865fa1672495396f2c477f90 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 1 Feb 2022 16:55:28 +0100 Subject: [PATCH 06/10] extra testing + minor refactor --- .../apm/agent/configuration/ServiceInfo.java | 28 ++++- .../agent/configuration/ServiceInfoTest.java | 100 +++++++++++++++++- .../apm/agent/impl/ElasticApmTracerTest.java | 46 +++++--- .../api/ElasticApmApiInstrumentationTest.java | 29 +++-- .../co/elastic/apm/servlet/tests/TestApp.java | 4 +- 5 files changed, 180 insertions(+), 27 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java index c2b301d2da..a02cfe3230 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java @@ -19,6 +19,7 @@ package co.elastic.apm.agent.configuration; import javax.annotation.Nullable; +import java.util.Objects; import java.util.Properties; import java.util.jar.Attributes; import java.util.jar.JarFile; @@ -28,8 +29,8 @@ public class ServiceInfo { private static final String JAR_VERSION_SUFFIX = "-(\\d+\\.)+(\\d+)(.*)?$"; private static final String DEFAULT_SERVICE_NAME = "unknown-java-service"; - private static final ServiceInfo EMPTY = ServiceInfo.of(null); private static final ServiceInfo AUTO_DETECTED = autoDetect(System.getProperties()); + private final String serviceName; @Nullable private final String serviceVersion; @@ -58,12 +59,12 @@ public static ServiceInfo of(@Nullable String serviceName) { public static ServiceInfo of(@Nullable String serviceName, @Nullable String serviceVersion) { if ((serviceName == null || serviceName.isEmpty()) && (serviceVersion == null || serviceVersion.isEmpty())) { - return ServiceInfo.empty(); + return empty(); } return new ServiceInfo(serviceName, serviceVersion); } - public static String replaceDisallowedServiceNameChars(String serviceName) { + private static String replaceDisallowedServiceNameChars(String serviceName) { return serviceName.replaceAll("[^a-zA-Z0-9 _-]", "-"); } @@ -192,4 +193,25 @@ public boolean hasServiceName() { public boolean isEmpty() { return !hasServiceName() && serviceVersion == null; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ServiceInfo that = (ServiceInfo) o; + return serviceName.equals(that.serviceName) && Objects.equals(serviceVersion, that.serviceVersion); + } + + @Override + public int hashCode() { + return Objects.hash(serviceName, serviceVersion); + } + + @Override + public String toString() { + return "ServiceInfo{" + + "name='" + serviceName + '\'' + + ", version='" + serviceVersion + '\'' + + '}'; + } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ServiceInfoTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ServiceInfoTest.java index 6a91096857..97418ce40c 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ServiceInfoTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ServiceInfoTest.java @@ -20,13 +20,18 @@ import org.junit.jupiter.api.Test; +import javax.annotation.Nullable; +import java.util.Map; import java.util.Properties; +import java.util.jar.Attributes; +import java.util.jar.Manifest; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.SoftAssertions.assertSoftly; class ServiceInfoTest { - private static String getDefaultServiceName(String sunJavaCommand) { + private static String getDefaultServiceName(@Nullable String sunJavaCommand) { Properties properties = new Properties(); if (sunJavaCommand != null) { properties.setProperty("sun.java.command", sunJavaCommand); @@ -88,4 +93,97 @@ void parseApplicationServers() { }); } + @Test + void testNormalizedName() { + checkServiceInfoEmpty(ServiceInfo.of("")); + checkServiceInfoEmpty(ServiceInfo.of(" ")); + + assertThat(ServiceInfo.of(" a")).isEqualTo(ServiceInfo.of("a")); + assertThat(ServiceInfo.of(" !web# ")).isEqualTo(ServiceInfo.of("-web-")); + } + + @Test + void createEmpty() { + checkServiceInfoEmpty(ServiceInfo.empty()); + assertThat(ServiceInfo.empty()) + .isEqualTo(ServiceInfo.empty()); + + } + + @Test + void of() { + checkServiceInfoEmpty(ServiceInfo.of(null)); + checkServiceInfoEmpty(ServiceInfo.of(null, null)); + + checkServiceInfo(ServiceInfo.of("service"), "service", null); + checkServiceInfo(ServiceInfo.of("service", null), "service", null); + checkServiceInfo(ServiceInfo.of("service", "1.2.3"), "service", "1.2.3"); + + } + + @Test + void checkEquality() { + checkEquality(ServiceInfo.of(null), ServiceInfo.empty()); + checkEquality(ServiceInfo.of(""), ServiceInfo.empty()); + checkEquality(ServiceInfo.of(null, null), ServiceInfo.empty()); + checkEquality(ServiceInfo.of("", ""), ServiceInfo.empty()); + } + + private static void checkEquality(ServiceInfo first, ServiceInfo second){ + assertThat(first) + .isEqualTo(second); + + assertThat(first.hashCode()) + .isEqualTo(second.hashCode()); + } + + @Test + void fromManifest() { + checkServiceInfoEmpty(ServiceInfo.fromManifest(null)); + checkServiceInfoEmpty(ServiceInfo.fromManifest(null)); + checkServiceInfoEmpty(ServiceInfo.fromManifest(new Manifest())); + + ServiceInfo serviceInfo = ServiceInfo.fromManifest(manifest(Map.of( + Attributes.Name.IMPLEMENTATION_TITLE.toString(), "service-name" + ))); + checkServiceInfo(serviceInfo, "service-name", null); + + serviceInfo = ServiceInfo.fromManifest(manifest(Map.of( + Attributes.Name.IMPLEMENTATION_TITLE.toString(), "my-service", + Attributes.Name.IMPLEMENTATION_VERSION.toString(), "v42" + ))); + checkServiceInfo(serviceInfo, "my-service", "v42"); + } + + private static Manifest manifest(Map entries) { + Manifest manifest = new Manifest(); + + Attributes attributes = manifest.getMainAttributes(); + entries.forEach(attributes::putValue); + + return manifest; + } + + private static void checkServiceInfoEmpty(ServiceInfo serviceInfo) { + assertThat(serviceInfo.isEmpty()).isTrue(); + assertThat(serviceInfo.getServiceName()).isEqualTo("unknown-java-service"); + assertThat(serviceInfo.hasServiceName()).isFalse(); + assertThat(serviceInfo.getServiceVersion()).isNull(); + + assertThat(serviceInfo).isEqualTo(ServiceInfo.empty()); + } + + private static void checkServiceInfo(ServiceInfo serviceInfo, String expectedServiceName, @Nullable String expectedServiceVersion) { + assertThat(serviceInfo.isEmpty()).isFalse(); + assertThat(serviceInfo.getServiceName()).isEqualTo(expectedServiceName); + assertThat(serviceInfo.hasServiceName()).isTrue(); + if (expectedServiceVersion == null) { + assertThat(serviceInfo.getServiceVersion()).isNull(); + } else { + assertThat(serviceInfo.getServiceVersion()).isEqualTo(expectedServiceVersion); + } + + assertThat(serviceInfo).isEqualTo(ServiceInfo.of(expectedServiceName, expectedServiceVersion)); + } + } 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 a0b7040af9..113f9328c4 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 @@ -431,12 +431,15 @@ void testOverrideServiceNameWithoutExplicitServiceName() { .configurationRegistry(SpyConfiguration.createSpyConfig()) .reporter(reporter) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden")); - startTestRootTransaction().end(); + ClassLoader cl = getClass().getClassLoader(); + ServiceInfo overridden = ServiceInfo.of("overridden"); + tracer.overrideServiceInfoForClassLoader(cl, overridden); + assertThat(tracer.getServiceInfo(cl)).isEqualTo(overridden); + startTestRootTransaction().end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden"); + checkServiceInfo(reporter.getFirstTransaction(), overridden); } @Test @@ -448,7 +451,9 @@ void testNotOverrideServiceNameWhenServiceNameConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden")); + ClassLoader cl = getClass().getClassLoader(); + tracer.overrideServiceInfoForClassLoader(cl, ServiceInfo.of("overridden")); + assertThat(tracer.getServiceInfo(cl)).isNull(); startTestRootTransaction().end(); @@ -467,11 +472,18 @@ void testNotOverrideServiceNameWhenDefaultServiceNameConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden", null)); + + ClassLoader cl = getClass().getClassLoader(); + tracer.overrideServiceInfoForClassLoader(cl, ServiceInfo.of("overridden")); + assertThat(tracer.getServiceInfo(cl)).isNull(); + startTestRootTransaction().end(); CoreConfiguration coreConfig = localConfig.getConfig(CoreConfiguration.class); - assertThat(ServiceInfo.autoDetect(System.getProperties()).getServiceName()).isEqualTo(coreConfig.getServiceName()); + + assertThat(ServiceInfo.autoDetect(System.getProperties())) + .isEqualTo(ServiceInfo.of(coreConfig.getServiceName())); + assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isNull(); if (command != null) { System.setProperty("sun.java.command", command); @@ -480,17 +492,24 @@ void testNotOverrideServiceNameWhenDefaultServiceNameConfigured() { } } + private static void checkServiceInfo(Transaction transaction, ServiceInfo expected) { + TraceContext traceContext = transaction.getTraceContext(); + assertThat(traceContext.getServiceName()).isEqualTo(expected.getServiceName()); + assertThat(traceContext.getServiceVersion()).isEqualTo(expected.getServiceVersion()); + } + @Test void testOverrideServiceVersionWithoutExplicitServiceVersion() { final ElasticApmTracer tracer = new ElasticApmTracerBuilder() .reporter(reporter) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); + + ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), overridden); startTestRootTransaction().end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); + checkServiceInfo(reporter.getFirstTransaction(), overridden); } @Test @@ -500,12 +519,15 @@ void testNotOverrideServiceVersionWhenServiceVersionConfigured() { .reporter(reporter) .configurationRegistry(localConfig) .buildAndStart(); - tracer.overrideServiceInfoForClassLoader(getClass().getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); + + ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); + ClassLoader cl = getClass().getClassLoader(); + tracer.overrideServiceInfoForClassLoader(cl, overridden); + assertThat(tracer.getServiceInfo(cl)).isEqualTo(overridden); startTestRootTransaction().end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); + checkServiceInfo(reporter.getFirstTransaction(), overridden); } @Test diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index 5bba9f49a0..980e1d3c79 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -24,6 +24,7 @@ import co.elastic.apm.agent.impl.TextHeaderMapAccessor; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.agent.impl.transaction.AbstractSpan; +import co.elastic.apm.agent.impl.transaction.TraceContext; import org.junit.jupiter.api.Test; import java.util.Collections; @@ -328,32 +329,40 @@ void testManualTimestampsDeactivated() { @Test void testOverrideServiceNameForClassLoader() { - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden")); + ServiceInfo overridden = ServiceInfo.of("overridden"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); ElasticApm.startTransaction().end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden"); + checkTransactionServiceInfo(overridden); } @Test void testOverrideServiceNameForClassLoaderWithRemoteParent() { - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden")); + ServiceInfo overridden = ServiceInfo.of("overridden"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden"); + checkTransactionServiceInfo(overridden); } @Test void testOverrideServiceVersionForClassLoader() { - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); + ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); ElasticApm.startTransaction().end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); + checkTransactionServiceInfo(overridden); } @Test void testOverrideServiceVersionForClassLoaderWithRemoteParent() { - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), ServiceInfo.of("overridden_name", "overridden_version")); + ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); + tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceName()).isEqualTo("overridden_name"); - assertThat(reporter.getFirstTransaction().getTraceContext().getServiceVersion()).isEqualTo("overridden_version"); + checkTransactionServiceInfo(overridden); + } + + private void checkTransactionServiceInfo(ServiceInfo expected){ + TraceContext traceContext = reporter.getFirstTransaction().getTraceContext(); + assertThat(traceContext.getServiceName()).isEqualTo(expected.getServiceName()); + assertThat(traceContext.getServiceVersion()).isEqualTo(expected.getServiceVersion()); } @Test 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 ae8f288c17..c99046b5c8 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 @@ -33,9 +33,10 @@ public abstract class TestApp { @Nullable private final String expectedServiceName; private final String deploymentContext; + @Nullable private final String expectedServiceVersion; - TestApp(String modulePath, String appFileName, String deploymentContext, String statusEndpoint, @Nullable String expectedServiceName, String expectedVersion) { + TestApp(String modulePath, String appFileName, String deploymentContext, String statusEndpoint, @Nullable String expectedServiceName, @Nullable String expectedVersion) { this.modulePath = modulePath; this.appFileName = appFileName; this.expectedServiceVersion = expectedVersion; @@ -94,6 +95,7 @@ public Map getAdditionalEnvVariables() { public abstract void test(AbstractServletContainerIntegrationTest test) throws Exception; + @Nullable public String getExpectedServiceVersion() { return expectedServiceVersion; } From c08ee4d35efdb092bb6cce7d5110e2e61029d687 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 1 Feb 2022 15:06:34 +0100 Subject: [PATCH 07/10] revert unintended changes --- .../elastic/apm/api/ElasticApmApiInstrumentationTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index 980e1d3c79..78183567bc 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -330,7 +330,7 @@ void testManualTimestampsDeactivated() { @Test void testOverrideServiceNameForClassLoader() { ServiceInfo overridden = ServiceInfo.of("overridden"); - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); + tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), overridden); ElasticApm.startTransaction().end(); checkTransactionServiceInfo(overridden); } @@ -338,7 +338,7 @@ void testOverrideServiceNameForClassLoader() { @Test void testOverrideServiceNameForClassLoaderWithRemoteParent() { ServiceInfo overridden = ServiceInfo.of("overridden"); - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); + tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), overridden); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); checkTransactionServiceInfo(overridden); } @@ -346,7 +346,7 @@ void testOverrideServiceNameForClassLoaderWithRemoteParent() { @Test void testOverrideServiceVersionForClassLoader() { ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); + tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), overridden); ElasticApm.startTransaction().end(); checkTransactionServiceInfo(overridden); } @@ -354,7 +354,7 @@ void testOverrideServiceVersionForClassLoader() { @Test void testOverrideServiceVersionForClassLoaderWithRemoteParent() { ServiceInfo overridden = ServiceInfo.of("overridden_name", "overridden_version"); - tracer.overrideServiceInfoForClassLoader(co.elastic.apm.agent.impl.transaction.Transaction.class.getClassLoader(), overridden); + tracer.overrideServiceInfoForClassLoader(Transaction.class.getClassLoader(), overridden); ElasticApm.startTransactionWithRemoteParent(key -> null).end(); checkTransactionServiceInfo(overridden); } From 05ae76ec4f4054f4d419f3efc1a2af2230f1e78d Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 1 Feb 2022 17:10:38 +0100 Subject: [PATCH 08/10] Use constant for ServiceInfo.EMPTY --- .../java/co/elastic/apm/agent/configuration/ServiceInfo.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java index a02cfe3230..35b9b880eb 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceInfo.java @@ -29,6 +29,7 @@ public class ServiceInfo { private static final String JAR_VERSION_SUFFIX = "-(\\d+\\.)+(\\d+)(.*)?$"; private static final String DEFAULT_SERVICE_NAME = "unknown-java-service"; + private static final ServiceInfo EMPTY = new ServiceInfo(null, null); private static final ServiceInfo AUTO_DETECTED = autoDetect(System.getProperties()); private final String serviceName; @@ -49,7 +50,7 @@ private ServiceInfo(@Nullable String serviceName, @Nullable String serviceVersio } public static ServiceInfo empty() { - return new ServiceInfo(null, null); + return EMPTY; } public static ServiceInfo of(@Nullable String serviceName) { From 0f444171c385547b468569f5f79c44b03649ac73 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 1 Feb 2022 20:02:40 +0100 Subject: [PATCH 09/10] Fix misnomer --- ...ervletWrappingControllerTransactionNameInstrumentation.java} | 2 +- .../services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/{ServletWrappingControllerServiceNameInstrumentation.java => ServletWrappingControllerTransactionNameInstrumentation.java} (96%) diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerServiceNameInstrumentation.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerTransactionNameInstrumentation.java similarity index 96% rename from apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerServiceNameInstrumentation.java rename to apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerTransactionNameInstrumentation.java index d23ae4b383..013a45ced3 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerServiceNameInstrumentation.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/springwebmvc/ServletWrappingControllerTransactionNameInstrumentation.java @@ -38,7 +38,7 @@ * to the name of the servlet, * overriding the transaction name set by {@link SpringTransactionNameInstrumentation} that would be {@code ServletWrappingController}. */ -public class ServletWrappingControllerServiceNameInstrumentation extends TracerAwareInstrumentation { +public class ServletWrappingControllerTransactionNameInstrumentation extends TracerAwareInstrumentation { @Override public ElementMatcher getTypeMatcher() { return named("org.springframework.web.servlet.mvc.ServletWrappingController"); diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation index 8e50e06722..c33046182c 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation @@ -1,5 +1,5 @@ co.elastic.apm.agent.springwebmvc.SpringTransactionNameInstrumentation -co.elastic.apm.agent.springwebmvc.ServletWrappingControllerServiceNameInstrumentation +co.elastic.apm.agent.springwebmvc.ServletWrappingControllerTransactionNameInstrumentation co.elastic.apm.agent.springwebmvc.ViewRenderInstrumentation co.elastic.apm.agent.springwebmvc.SpringServiceNameInstrumentation co.elastic.apm.agent.springwebmvc.ExceptionHandlerInstrumentation From a6249ccbe1ae013861e6bfe57dc4e6095feff360 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 2 Feb 2022 09:09:56 +0100 Subject: [PATCH 10/10] Add changelog --- CHANGELOG.asciidoc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 27e5901dad..3afb0293dd 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -23,16 +23,23 @@ endif::[] [[release-notes-1.28.5]] ==== 1.28.5 - YYYY/MM/DD +[float] +===== Breaking changes +* Changes in service name auto-discovery of jar files (see Features section) + [float] ===== Features * Exceptions that are logged using the fatal log level are now captured (log4j2 only) - {pull}2377[#2377] * Replaced `authorization` in the default value of `sanitize_field_names` with `*auth*` - {pull}2326[#2326] * Unsampled transactions are dropped and not sent to the APM-Server if the APM-Server version is 8.0+ - {pull}2329[#2329] * Adding agent logging capabilities to our SDK, making it available for external plugins - {pull}2390[#2390] -* When the `MANIFEST.MF` of the main jar contains the `Implementation-Title` attribute, it is used as the default service name - {pull}1921[#1921] - Note: this may change your service names if you relied on the auto-discovery that uses the name of the jar file. If that jar file also contains an `Implementation-Title` attribute in the `MANIFEST.MF` file, the latter will take precedence. -* When the `MANIFEST.MF` of the main jar contains the `Implementation-Version` attribute, it is used as the default service version (except for application servers) - {pull}1922[#1922] -* Added support for overwritting the service version per classloader - {pull}1726[#1726] +* Service name auto-discovery improvements +** For applications deployed to application servers (`war` files) and standalone jars that are started with `java -jar`, + the agent now discovers the `META-INF/MANIFEST.MF` file. +** If the manifest contains the `Implementation-Title` attribute, it is used as the default service name - {pull}1921[#1921], {pull}2434[#2434] + + *Note*: this may change your service names if you relied on the auto-discovery that uses the name of the jar file. + If that jar file also contains an `Implementation-Title` attribute in the `MANIFEST.MF` file, the latter will take precedence. +** When the manifest contains the `Implementation-Version` attribute, it is used as the default service version - {pull}1726[#1726], {pull}1922[#1922], {pull}2434[#2434] * Added support for instrumenting Struts 2 static resource requests - {pull}1949[#1949] * Added support for Java/Jakarta WebSocket ServerEndpoint - {pull}2281[#2281] * Added support for setting the service name on Log4j2's EcsLayout - {pull}2296[#2296]