From d1e66abd591d299f3daa69f6e7f601feb55e56f4 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 13 Jun 2022 10:12:58 +0200 Subject: [PATCH 1/4] JAX-W @WebMethod is optional --- .../bci/bytebuddy/MethodHierarchyMatcher.java | 65 ++++++++++++------- ...xWsTransactionNameInstrumentationTest.java | 3 - .../JaxWsTransactionNameInstrumentation.java | 13 ++-- .../apm/soap/BaseHelloWorldService.java | 23 +++++++ .../elastic/apm/soap/HelloWorldService.java | 5 +- 5 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 integration-tests/soap-test/src/main/java/co/elastic/apm/soap/BaseHelloWorldService.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java index 6ec9f282ff..c800bc6abb 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java @@ -20,8 +20,14 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.description.type.TypeList; import net.bytebuddy.matcher.ElementMatcher; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; + +import static net.bytebuddy.matcher.ElementMatchers.any; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; import static net.bytebuddy.matcher.ElementMatchers.is; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,45 +45,60 @@ public class MethodHierarchyMatcher extends ElementMatcher.Junction.AbstractBase private final ElementMatcher 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 { } From 32a85f10a687c15edf8338b3501b54d6b67edd7b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 13 Jun 2022 10:28:24 +0200 Subject: [PATCH 2/4] Missing type info --- .../elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java index c800bc6abb..3d4955c995 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/MethodHierarchyMatcher.java @@ -67,7 +67,7 @@ public MethodHierarchyMatcher whereHierarchyContains(ElementMatcher()); + return declaresInHierarchy(targetMethod, targetMethod.getDeclaringType().asErasure(), new ArrayDeque()); } private boolean declaresInHierarchy(MethodDescription targetMethod, TypeDescription type, Deque hierarchy) { From 7323c28062dae2ecfc36eb745623537533bcabba Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 13 Jun 2022 11:43:56 +0200 Subject: [PATCH 3/4] Add changelog --- CHANGELOG.asciidoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 86cfd1f742..625d9f913c 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -52,6 +52,7 @@ from maven at runtime unless configured otherwise through `--download-agent-ver ===== Bug fixes * Fix missing attributes in bridged OTel transactions - {pull}2657[#2657] * Fix `transaction.result` with bridged OTel transactions - {pull}2660[#2660] +* Fix for JAX-WS (SOAP) transaction names. Now the agent also 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 @@ -997,7 +998,7 @@ for more details) - {pull}1009[#1009] <> config option] * Add ability to ignore some exceptions to be reported as errors < Date: Mon, 13 Jun 2022 15:23:58 +0200 Subject: [PATCH 4/4] Move changelog entry to 1.32.1 --- CHANGELOG.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1cdeab559d..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 @@ -67,7 +68,6 @@ from maven at runtime unless configured otherwise through `--download-agent-ver ===== Bug fixes * Fix missing attributes in bridged OTel transactions - {pull}2657[#2657] * Fix `transaction.result` with bridged OTel transactions - {pull}2660[#2660] -* Fix for JAX-WS (SOAP) transaction names. Now the agent also properly names transaction for web service methods that are not annotated with `@WebMethod`. - {pull}2667[#2667] [[release-notes-1.31.0]] ==== 1.31.0 - 2022/05/17