diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 99837ea8c3..f3dc3c4797 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,7 @@ endif::[] [float] ===== Bug fixes +* Fix for JAX-WS (SOAP) transaction names. The agent now properly names transaction for web service methods that are not annotated with `@WebMethod`. - {pull}2667[#2667] [[release-notes-1.x]] === Java Agent version 1.x @@ -1009,7 +1010,7 @@ for more details) - {pull}1009[#1009] <> config option] * Add ability to ignore some exceptions to be reported as errors < extraMethodMatcher; private final ElementMatcher superClassMatcher; + private final ElementMatcher hierarchyMatcher; MethodHierarchyMatcher(ElementMatcher extraMethodMatcher) { - this(extraMethodMatcher, not(is(TypeDescription.ForLoadedType.OBJECT))); + this(extraMethodMatcher, not(is(TypeDescription.ForLoadedType.OBJECT)), any()); } - private MethodHierarchyMatcher(ElementMatcher extraMethodMatcher, ElementMatcher superClassMatcher) { + private MethodHierarchyMatcher(ElementMatcher extraMethodMatcher, ElementMatcher superClassMatcher, ElementMatcher hierachyMatcher) { this.extraMethodMatcher = extraMethodMatcher; this.superClassMatcher = superClassMatcher; + this.hierarchyMatcher = hierachyMatcher; + } + + public MethodHierarchyMatcher onSuperClassesThat(ElementMatcher superClassMatcher) { + return new MethodHierarchyMatcher(extraMethodMatcher, superClassMatcher, hierarchyMatcher); } - public ElementMatcher onSuperClassesThat(ElementMatcher superClassMatcher) { - return new MethodHierarchyMatcher(extraMethodMatcher, superClassMatcher); + public MethodHierarchyMatcher whereHierarchyContains(ElementMatcher hierarchyMatcher) { + return new MethodHierarchyMatcher(extraMethodMatcher, superClassMatcher, hierarchyMatcher); } @Override public boolean matches(MethodDescription targetMethod) { - return declaresInHierarchy(targetMethod, targetMethod.getDeclaringType().asErasure()); + return declaresInHierarchy(targetMethod, targetMethod.getDeclaringType().asErasure(), new ArrayDeque()); } - private boolean declaresInHierarchy(MethodDescription targetMethod, TypeDescription type) { - if (declaresMethod(named(targetMethod.getName()) - .and(returns(targetMethod.getReturnType().asErasure())) - .and(takesArguments(targetMethod.getParameters().asTypeList().asErasures())) - .and(extraMethodMatcher)) - .matches(type)) { - return true; - } - for (TypeDescription interfaze : type.getInterfaces().asErasures()) { - if (superClassMatcher.matches(interfaze)) { - if (declaresInHierarchy(targetMethod, interfaze)) { - return true; + private boolean declaresInHierarchy(MethodDescription targetMethod, TypeDescription type, Deque hierarchy) { + hierarchy.push(type); + try { + if (declaresMethod(named(targetMethod.getName()) + .and(returns(targetMethod.getReturnType().asErasure())) + .and(takesArguments(targetMethod.getParameters().asTypeList().asErasures())) + .and(extraMethodMatcher)) + .matches(type) + && !new TypeList.Explicit(new ArrayList<>(hierarchy)) + .filter(hierarchyMatcher) + .isEmpty() + ) { + return true; + } + for (TypeDescription interfaze : type.getInterfaces().asErasures()) { + if (superClassMatcher.matches(interfaze)) { + if (declaresInHierarchy(targetMethod, interfaze, hierarchy)) { + return true; + } } } + final TypeDescription.Generic superClass = type.getSuperClass(); + if (superClass != null && superClassMatcher.matches(superClass.asErasure())) { + return declaresInHierarchy(targetMethod, superClass.asErasure(), hierarchy); + } + return false; + } finally { + hierarchy.pop(); } - final TypeDescription.Generic superClass = type.getSuperClass(); - if (superClass != null && superClassMatcher.matches(superClass.asErasure())) { - return declaresInHierarchy(targetMethod, superClass.asErasure()); - } - return false; } } diff --git a/apm-agent-plugins/apm-jaxws-plugin-jakartaee-test/src/test/java/co/elastic/apm/agent/jaxws/JakartaeeJaxWsTransactionNameInstrumentationTest.java b/apm-agent-plugins/apm-jaxws-plugin-jakartaee-test/src/test/java/co/elastic/apm/agent/jaxws/JakartaeeJaxWsTransactionNameInstrumentationTest.java index 8a9e7c671c..b0722501a2 100644 --- a/apm-agent-plugins/apm-jaxws-plugin-jakartaee-test/src/test/java/co/elastic/apm/agent/jaxws/JakartaeeJaxWsTransactionNameInstrumentationTest.java +++ b/apm-agent-plugins/apm-jaxws-plugin-jakartaee-test/src/test/java/co/elastic/apm/agent/jaxws/JakartaeeJaxWsTransactionNameInstrumentationTest.java @@ -18,7 +18,6 @@ */ package co.elastic.apm.agent.jaxws; -import jakarta.jws.WebMethod; import jakarta.jws.WebService; import jakarta.jws.soap.SOAPBinding; import org.junit.jupiter.api.BeforeEach; @@ -33,8 +32,6 @@ void setUp() { @SOAPBinding(style = SOAPBinding.Style.RPC) @WebService(targetNamespace = "elastic") public interface HelloWorldService extends BaseHelloWorldService { - @WebMethod - String sayHello(); } @WebService(serviceName = "HelloWorldService", portName = "HelloWorld", name = "HelloWorld", diff --git a/apm-agent-plugins/apm-jaxws-plugin/src/main/java/co/elastic/apm/agent/jaxws/JaxWsTransactionNameInstrumentation.java b/apm-agent-plugins/apm-jaxws-plugin/src/main/java/co/elastic/apm/agent/jaxws/JaxWsTransactionNameInstrumentation.java index 3e84f0075a..c3753655ec 100644 --- a/apm-agent-plugins/apm-jaxws-plugin/src/main/java/co/elastic/apm/agent/jaxws/JaxWsTransactionNameInstrumentation.java +++ b/apm-agent-plugins/apm-jaxws-plugin/src/main/java/co/elastic/apm/agent/jaxws/JaxWsTransactionNameInstrumentation.java @@ -40,7 +40,9 @@ import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_HIGH_LEVEL_FRAMEWORK; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader; +import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy; import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -59,7 +61,7 @@ public static class AdviceClass { public static void setTransactionName(@SimpleMethodSignature String signature) { final Transaction transaction = tracer.currentTransaction(); if (transaction != null) { - transaction.withName(signature, PRIO_HIGH_LEVEL_FRAMEWORK); + transaction.withName(signature, PRIO_HIGH_LEVEL_FRAMEWORK, false); transaction.setFrameworkName(FRAMEWORK_NAME); } } @@ -90,9 +92,12 @@ public ElementMatcher.Junction getClassLoaderMatcher() { @Override public ElementMatcher getMethodMatcher() { return overridesOrImplementsMethodThat( - isAnnotatedWith( - namedOneOf("javax.jws.WebMethod", "jakarta.jws.WebMethod"))) - .onSuperClassesThat(isInAnyPackage(applicationPackages, ElementMatchers.any())); + isPublic().and(isDeclaredBy(isInterface())) + ).whereHierarchyContains( + isInterface().and(isAnnotatedWith(namedOneOf("javax.jws.WebService", "jakarta.jws.WebService"))) + ).onSuperClassesThat( + isInAnyPackage(applicationPackages, ElementMatchers.any()) + ); } @Override diff --git a/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/BaseHelloWorldService.java b/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/BaseHelloWorldService.java new file mode 100644 index 0000000000..7dcae5e99f --- /dev/null +++ b/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/BaseHelloWorldService.java @@ -0,0 +1,23 @@ +/* + * 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.soap; + +public interface BaseHelloWorldService { + String sayHello(); +} diff --git a/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/HelloWorldService.java b/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/HelloWorldService.java index e7460ed4c2..a7e3b17d21 100644 --- a/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/HelloWorldService.java +++ b/integration-tests/soap-test/src/main/java/co/elastic/apm/soap/HelloWorldService.java @@ -18,13 +18,10 @@ */ package co.elastic.apm.soap; -import javax.jws.WebMethod; import javax.jws.WebService; import javax.jws.soap.SOAPBinding; @SOAPBinding(style = SOAPBinding.Style.RPC) @WebService(targetNamespace = "elastic") -public interface HelloWorldService { - @WebMethod - String sayHello(); +public interface HelloWorldService extends BaseHelloWorldService { }