Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1009,7 +1010,7 @@ for more details) - {pull}1009[#1009]
<<config-service-node-name, `service_node_name`>>
config option]
* Add ability to ignore some exceptions to be reported as errors <<config-ignore-exceptions[ignore_exceptions]
* Applying new logic for JMS `javax.jms.MessageConsumer#receive` so that, instead of the transaction created for the
* Applying new logic for JMS `javax.jms.MessageConsumer#receive` so that, instead of the transaction created for the
polling method itself (ie from `receive` start to end), the agent will create a transaction attempting to capture
the code executed during actual message handling.
This logic is suitable for environments where polling APIs are invoked within dedicated polling threads.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,45 +45,60 @@ public class MethodHierarchyMatcher extends ElementMatcher.Junction.AbstractBase

private final ElementMatcher<? super MethodDescription> extraMethodMatcher;
private final ElementMatcher<? super TypeDescription> superClassMatcher;
private final ElementMatcher<? super TypeDescription> hierarchyMatcher;

MethodHierarchyMatcher(ElementMatcher<? super MethodDescription> extraMethodMatcher) {
this(extraMethodMatcher, not(is(TypeDescription.ForLoadedType.OBJECT)));
this(extraMethodMatcher, not(is(TypeDescription.ForLoadedType.OBJECT)), any());
}

private MethodHierarchyMatcher(ElementMatcher<? super MethodDescription> extraMethodMatcher, ElementMatcher<? super TypeDescription> superClassMatcher) {
private MethodHierarchyMatcher(ElementMatcher<? super MethodDescription> extraMethodMatcher, ElementMatcher<? super TypeDescription> superClassMatcher, ElementMatcher<? super TypeDescription> hierachyMatcher) {
this.extraMethodMatcher = extraMethodMatcher;
this.superClassMatcher = superClassMatcher;
this.hierarchyMatcher = hierachyMatcher;
}

public MethodHierarchyMatcher onSuperClassesThat(ElementMatcher<? super TypeDescription> superClassMatcher) {
return new MethodHierarchyMatcher(extraMethodMatcher, superClassMatcher, hierarchyMatcher);
}

public ElementMatcher<MethodDescription> onSuperClassesThat(ElementMatcher<? super TypeDescription> superClassMatcher) {
return new MethodHierarchyMatcher(extraMethodMatcher, superClassMatcher);
public MethodHierarchyMatcher whereHierarchyContains(ElementMatcher<? super TypeDescription> 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<TypeDescription>());
}

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<TypeDescription> 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()
Comment on lines +81 to +83

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] this one probably deserves to be explained a bit, maybe through a comment. If I understand it correctly it's "the type matches the hierarchyMatcher", maybe nesting an IF in this case could help.

) {
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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}
Expand Down Expand Up @@ -90,9 +92,12 @@ public ElementMatcher.Junction<ClassLoader> getClassLoaderMatcher() {
@Override
public ElementMatcher<? super MethodDescription> getMethodMatcher() {
return overridesOrImplementsMethodThat(
isAnnotatedWith(
namedOneOf("javax.jws.WebMethod", "jakarta.jws.WebMethod")))
.onSuperClassesThat(isInAnyPackage(applicationPackages, ElementMatchers.<NamedElement>any()));
isPublic().and(isDeclaredBy(isInterface()))
).whereHierarchyContains(
isInterface().and(isAnnotatedWith(namedOneOf("javax.jws.WebService", "jakarta.jws.WebService")))
).onSuperClassesThat(
isInAnyPackage(applicationPackages, ElementMatchers.<NamedElement>any())
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] maybe add a small comment to remind that the @WebMethod annotation is optional on the method level.

}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
}