Fix ignore on request path

CHANGELOG.asciidoc

@@ -23,6 +23,17 @@ endif::[]
 [[release-notes-1.27.0]]
 ==== 1.27.0 - YYYY/MM/DD
 
+[float]
+===== Potentially breaking changes
+* `transaction_ignore_urls` now relies on full request URL path - {pull}2146[#2146]
+** On a typical application server like Tomcat, deploying an `app.war` application to the non-ROOT context makes it accessible with `http://localhost:8080/app/`
+** Ignoring the whole webapp through `/app/*` was not possible until now.
+** Existing configuration may need to be updated to include the deployment context, thus for example `/static/*.js` used to
+exclude known static files in all applications might be changed to `/app/static/*.js` or `*/static/*.js`.
+** It only impacts prefix patterns due to the additional context path in pattern.
+** It does not impact deployment within the `ROOT` context like Spring-boot which do not have such context path prefix.
+
+
 [float]
 ===== Features
 * Improved capturing of logged exceptions when using Log4j2 {pull}2139[#2139]

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java

@@ -60,33 +60,22 @@ public Transaction createAndActivateTransaction(HttpServletRequest request) {
  return transaction;
  }
 
- private boolean isExcluded(HttpServletRequest request) {
+ boolean isExcluded(HttpServletRequest request) {
  String userAgent = request.getHeader("User-Agent");
 
- String pathFirstPart = request.getServletPath();
- String pathSecondPart = Objects.toString(request.getPathInfo(), "");
+ String requestUri = request.getRequestURI();
 
- if (pathFirstPart.isEmpty()) {
- // when servlet path is empty, reconstructing the path from the request URI
- // this can happen when transaction is created by a filter (and thus servlet path is unknown yet)
- String contextPath = request.getContextPath();
- if (null != contextPath) {
- pathFirstPart = request.getRequestURI().substring(contextPath.length());
- pathSecondPart = "";
- }
- }
-
- final WildcardMatcher excludeUrlMatcher = WildcardMatcher.anyMatch(webConfiguration.getIgnoreUrls(), pathFirstPart, pathSecondPart);
+ 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 {}", pathFirstPart, pathSecondPart, excludeUrlMatcher);
+ 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: {}", pathFirstPart, pathSecondPart, userAgent);
+ logger.trace("No matcher found for excluding this request with URL: {}, and User-Agent: {}", requestUri, userAgent);
  }
  return isExcluded;
  }

apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java

@@ -46,10 +46,11 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Objects;
-import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_USER_SUPPLIED;
 import static org.assertj.core.api.Java6Assertions.assertThat;
@@ -135,60 +136,41 @@ void testIgnoreUrlStartWithNoMatch() throws IOException, ServletException {
 
  @Test
  void testIgnoreUrl() {
- Arrays.asList("/resources*", "*.js").forEach(ignoreConfig -> {
- // we have 3 different cases:
- // 1. servlet path without path info (null)
- testIgnoreUrl(ignoreConfig, r -> r.setServletPath("/resources/test.js"));
- // 2. servlet path with path info
- testIgnoreUrl(ignoreConfig, r -> {
- r.setServletPath("/resources");
- r.setPathInfo("/test.js");
- });
- // 3. empty servlet path, when servlet path is unknown yet (filters)
- testIgnoreUrl(ignoreConfig, r -> {
- r.setServletPath("");
- r.setRequestURI("/context/resources/test.js");
- r.setContextPath("/context");
- });
- });
+ List<WildcardMatcher> config = Stream.of("/context/resources*", "*.js").map(WildcardMatcher::valueOf).collect(Collectors.toList());
+ testIgnoreUrl(config, "/context/resources/test.xml");
+ testIgnoreUrl(config, "/ignored.js");
  }
 
- void testIgnoreUrl(String ignoreUrl, Consumer<MockHttpServletRequest> setRequestProperties) {
+ void testIgnoreUrl(List<WildcardMatcher> ignoreConfig, String requestUri) {
  reset(webConfiguration);
  filterChain = new MockFilterChain(new HttpServlet() {
  });
- when(webConfiguration.getIgnoreUrls()).thenReturn(Collections.singletonList(WildcardMatcher.valueOf(ignoreUrl)));
+ when(webConfiguration.getIgnoreUrls()).thenReturn(ignoreConfig);
  final MockHttpServletRequest request = new MockHttpServletRequest();
- setRequestProperties.accept(request);
+ request.setRequestURI(requestUri);
  try {
  filterChain.doFilter(request, new MockHttpServletResponse());
  } catch (ServletException|IOException e) {
  throw new IllegalStateException(e);
  }
  verify(webConfiguration, times(1)).getIgnoreUrls();
  assertThat(reporter.getTransactions())
- .describedAs("request should be ignored by config = %s", ignoreUrl)
+ .describedAs("request with URI %s should be ignored by config = %s", requestUri, ignoreConfig)
  .hasSize(0);
  }
 
- void testIgnoreUserAgent(String ignoreUserAgent, String userAgent) throws ServletException, IOException {
- when(webConfiguration.getIgnoreUserAgents()).thenReturn(Collections.singletonList(WildcardMatcher.valueOf(ignoreUserAgent)));
+ @Test
+ void testIgnoreUserAGent() throws IOException, ServletException {
+ String config = "curl/*";
+ String userAgent = "curl/7.54.0";
+
+ when(webConfiguration.getIgnoreUserAgents()).thenReturn(Collections.singletonList(WildcardMatcher.valueOf(config)));
  final MockHttpServletRequest request = new MockHttpServletRequest();
  request.addHeader("user-agent", userAgent);
  filterChain.doFilter(request, new MockHttpServletResponse());
  assertThat(reporter.getTransactions()).hasSize(0);
  }
 
- @Test
- void testIgnoreUserAgentStartWith() throws IOException, ServletException {
- testIgnoreUserAgent("curl/*","curl/7.54.0");
- }
-
- @Test
- void testIgnoreUserAgentInfix() throws IOException, ServletException {
- testIgnoreUserAgent("*pingdom*","Pingdom.com_bot_version_1.4_(http://www.pingdom.com)");
- }
-
  @Test
  void testDoNotOverrideUsername() throws IOException, ServletException {
  filterChain = new MockFilterChain(new HttpServlet() {

apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ServletTransactionHelperTest.java

@@ -20,15 +20,12 @@
 
 import co.elastic.apm.agent.AbstractInstrumentationTest;
 import co.elastic.apm.agent.MockTracer;
-import co.elastic.apm.agent.configuration.SpyConfiguration;
-import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
+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.impl.context.web.WebConfiguration;
 import co.elastic.apm.agent.util.TransactionNameUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.stagemonitor.configuration.ConfigurationRegistry;
 
 import javax.annotation.Nonnull;
 import java.net.URL;
@@ -46,12 +43,8 @@ class ServletTransactionHelperTest extends AbstractInstrumentationTest {
 
  @BeforeEach
  void setUp() {
- ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
  webConfig = config.getConfig(WebConfiguration.class);
- servletTransactionHelper = new ServletTransactionHelper(new ElasticApmTracerBuilder()
- .configurationRegistry(config)
- .reporter(reporter)
- .build());
+ servletTransactionHelper = new ServletTransactionHelper(tracer);
  }
 
  @Test
@@ -123,4 +116,5 @@ private String getTransactionName(String method, String path) {
  servletTransactionHelper.applyDefaultTransactionName(method, path, null, transaction);
  return transaction.getNameAsString();
  }
+
 }

apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperTest.java

@@ -0,0 +1,129 @@
+/*
+ * 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.AbstractInstrumentationTest;
+import co.elastic.apm.agent.impl.context.web.WebConfiguration;
+import co.elastic.apm.agent.matcher.WildcardMatcher;
+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;
+
+ @BeforeEach
+ void setUp() {
+ webConfig = config.getConfig(WebConfiguration.class);
+ helper = new ServletTransactionCreationHelper(tracer);
+ }
+
+ @ParameterizedTest
+ @CsvSource(delimiterString = " ", value = {
+ "/not-ignored ",
+ "/ ",
+ " ",
+ "/index.html *.xml"})
+ void requestPathNotIgnored(String path, String ignoreExpr) {
+ checkRequestPathIgnored(path, ignoreExpr, false);
+ }
+
+ @ParameterizedTest
+ @CsvSource(delimiterString = " ", value = {
+ "/ignored/from/prefix /ignored*",
+ "/ignored/with-suffix.js *.js",
+ "/ignored/with-suffix.html *.js,*.html",
+ "/ignored/with/term *with*"})
+ void requestPathIgnored(String path, String ignoreExpr) {
+ checkRequestPathIgnored(path, ignoreExpr, true);
+ }
+
+ 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);
+ assertThat(isIgnored)
+ .describedAs("request with path '%s' %s be ignored", expectIgnored ? "should" : "should not", path)
+ .isEqualTo(expectIgnored);
+
+ }
+
+ @ParameterizedTest
+ @CsvSource(delimiterString = " ", value = {
+ "anderson and*",
+ "anderson *son",
+ "anderson *der*",
+ "anderson smith,anderson"
+ })
+ 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))
+ .describedAs("request with user-agent '%s' should be ignored", userAgent)
+ .isTrue();
+ }
+
+ private static List<WildcardMatcher> parseWildcard(@Nullable String expr) {
+ if (null == expr || expr.isEmpty()) {
+ return Collections.emptyList();
+ }
+ return Stream.of(expr.split(","))
+ .map(WildcardMatcher::valueOf)
+ .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);
+ }
+}

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java

@@ -152,10 +152,18 @@ protected AbstractServletContainerIntegrationTest(GenericContainer<?> servletCon
  }
  this.expectedDefaultServiceName = expectedDefaultServiceName;
  this.containerName = containerName;
+
+ List<String> ignoreUrls = new ArrayList<>();
+ for (TestApp app : getTestApps()) {
+ ignoreUrls.add(String.format("/%s/status*", app.getDeploymentContext()));
+ }
+ ignoreUrls.add("/favicon.ico");
+ String ignoreUrlConfig = String.join(",", ignoreUrls);
+
  servletContainer
  .withNetwork(Network.SHARED)
  .withEnv("ELASTIC_APM_SERVER_URL", "http://apm-server:1080")
- .withEnv("ELASTIC_APM_IGNORE_URLS", "/status*,/favicon.ico")
+ .withEnv("ELASTIC_APM_IGNORE_URLS", ignoreUrlConfig)
  .withEnv("ELASTIC_APM_REPORT_SYNC", "true")
  .withEnv("ELASTIC_APM_LOG_LEVEL", "DEBUG")
  .withEnv("ELASTIC_APM_METRICS_INTERVAL", "1s")

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiApplicationServerTestApp.java

@@ -29,7 +29,11 @@
 public class CdiApplicationServerTestApp extends TestApp {
 
  public CdiApplicationServerTestApp() {
- super("../cdi-app/cdi-app-dependent", "cdi-app.war", "/cdi-app/status.html", "CDI App");
+ super("../cdi-app/cdi-app-dependent",
+ "cdi-app.war",
+ "cdi-app",
+ "status.html",
+ "CDI App");
  }
 
  @Override

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/CdiServletContainerTestApp.java

@@ -23,7 +23,11 @@
 public class CdiServletContainerTestApp extends TestApp {
 
  public CdiServletContainerTestApp() {
- super("../cdi-app/cdi-app-standalone", "cdi-app.war", "/cdi-app/status.html", "CDI App");
+ super("../cdi-app/cdi-app-standalone",
+ "cdi-app.war",
+ "cdi-app",
+ "status.html",
+ "CDI App");
  }
 
  @Override

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ExternalPluginTestApp.java

@@ -31,10 +31,10 @@
 
 public class ExternalPluginTestApp extends TestApp {
  public ExternalPluginTestApp() {
- super(
- "../external-plugin-test/external-plugin-app",
+ super("../external-plugin-test/external-plugin-app",
  "external-plugin-webapp.war",
- "/external-plugin-webapp/status.html",
+ "external-plugin-webapp",
+ "status.html",
  "external-plugin-webapp"
  );
  }

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java

@@ -30,7 +30,11 @@
 public class JsfApplicationServerTestApp extends TestApp {
 
  public JsfApplicationServerTestApp() {
- super("../jsf-app/jsf-app-dependent", "jsf-http-get.war", "/jsf-http-get/status.html", "jsf-http-get");
+ super("../jsf-app/jsf-app-dependent",
+ "jsf-http-get.war",
+ "jsf-http-get",
+ "status.html",
+ "jsf-http-get");
  }
 
  @Override

integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfServletContainerTestApp.java

@@ -22,7 +22,11 @@
 
 public class JsfServletContainerTestApp extends TestApp {
  public JsfServletContainerTestApp() {
- super("../jsf-app/jsf-app-standalone", "jsf-http-get.war", "/jsf-http-get/status.html", "jsf-http-get");
+ super("../jsf-app/jsf-app-standalone",
+ "jsf-http-get.war",
+ "jsf-http-get",
+ "status.html",
+ "jsf-http-get");
  }
 
  @Override