Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance/document msg listener heuristic #3299

Merged
merged 10 commits into from
Aug 25, 2023
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
[float]
===== Features
* Virtual thread support - {pull}3244[#3244], {pull}3286[#3286]
* Include `application_packages` in JMS listener naming heuristic - {pull}3299[#3299]

[float]
===== Bug fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,21 @@
import co.elastic.apm.agent.bci.ElasticApmAgent;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.Tracer;
import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration;
import co.elastic.apm.agent.impl.transaction.Transaction;
import co.elastic.apm.agent.jms.jakarta.test.TestMessageConsumer;
import co.elastic.apm.agent.jms.jakarta.test.TestMessageListener;
import co.elastic.apm.agent.jms.jakarta.test.TestMsgHandler;
import testapp.TestMessageConsumer;
import testapp.TestMessageListener;
import testapp.TestMsgHandler;
import co.elastic.apm.agent.tracer.GlobalTracer;
import co.elastic.apm.agent.tracer.configuration.MessagingConfiguration;
import net.bytebuddy.agent.ByteBuddyAgent;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.*;
import org.stagemonitor.configuration.ConfigurationRegistry;

import jakarta.jms.Message;
import jakarta.jms.MessageListener;
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicReference;

import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat;
Expand Down Expand Up @@ -70,9 +70,11 @@ private void startAgent(){

@Test
public void testJmsMessageListenerPackage_defaultConfig() throws Exception {
// default configuration
doReturn(Collections.emptyList()).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();

startAgent();

// default configuration
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.MATCHING_NAME_CONVENTION);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.INNER_CLASS);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.LAMBDA);
Expand All @@ -81,7 +83,20 @@ public void testJmsMessageListenerPackage_defaultConfig() throws Exception {

@Test
public void testJmsMessageListenerPackage_customValue() throws Exception {
doReturn(Arrays.asList("co.elastic.apm.agent.jms.jakarta.test")).when(config.getConfig(MessagingConfiguration.class)).getJmsListenerPackages();
doReturn(Collections.emptyList()).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();
doReturn(Arrays.asList("testapp")).when(config.getConfig(MessagingConfiguration.class)).getJmsListenerPackages();

startAgent();

testJmsMessageListenerPackage(true, JmsMessageListenerVariant.MATCHING_NAME_CONVENTION);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.INNER_CLASS);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.LAMBDA);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.NOT_MATCHING_NAME_CONVENTION);
}

@Test
public void testJmsMessageListenerPackage_applicationPackages() throws Exception {
doReturn(Arrays.asList("testapp")).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();

startAgent();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.jakarta.test;
package testapp;

import jakarta.jms.JMSException;
import jakarta.jms.Message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.jakarta.test;
package testapp;

import co.elastic.apm.agent.impl.Tracer;
import co.elastic.apm.agent.impl.transaction.Transaction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.jakarta.test;
package testapp;


import co.elastic.apm.agent.impl.Tracer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@

public class JmsMessageListenerInstrumentation extends BaseJmsInstrumentation {

@SuppressWarnings("WeakerAccess")
public static final Logger logger = LoggerFactory.getLogger(JmsMessageListenerInstrumentation.class);

@Override
public ElementMatcher<? super NamedElement> getTypeMatcherPreFilter() {
return getMessageListenerTypeMatcherPreFilter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@

import co.elastic.apm.agent.MockTracer;
import co.elastic.apm.agent.bci.ElasticApmAgent;
import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration;
import co.elastic.apm.agent.tracer.configuration.MessagingConfiguration;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.Tracer;
import co.elastic.apm.agent.tracer.GlobalTracer;
import co.elastic.apm.agent.impl.transaction.Transaction;
import co.elastic.apm.agent.jms.javax.test.TestMessageConsumer;
import co.elastic.apm.agent.jms.javax.test.TestMessageListener;
import co.elastic.apm.agent.jms.javax.test.TestMsgHandler;
import testapp.TestMessageConsumer;
import testapp.TestMessageListener;
import testapp.TestMsgHandler;
import net.bytebuddy.agent.ByteBuddyAgent;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -37,6 +38,7 @@
import javax.jms.Message;
import javax.jms.MessageListener;
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicReference;

import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat;
Expand Down Expand Up @@ -70,9 +72,11 @@ private void startAgent(){

@Test
public void testJmsMessageListenerPackage_defaultConfig() throws Exception {
// default configuration
doReturn(Collections.emptyList()).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();

startAgent();

// default configuration
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.MATCHING_NAME_CONVENTION);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.INNER_CLASS);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.LAMBDA);
Expand All @@ -81,7 +85,20 @@ public void testJmsMessageListenerPackage_defaultConfig() throws Exception {

@Test
public void testJmsMessageListenerPackage_customValue() throws Exception {
doReturn(Arrays.asList("co.elastic.apm.agent.jms.javax.test")).when(config.getConfig(MessagingConfiguration.class)).getJmsListenerPackages();
doReturn(Collections.emptyList()).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();
doReturn(Arrays.asList("testapp")).when(config.getConfig(MessagingConfiguration.class)).getJmsListenerPackages();

startAgent();

testJmsMessageListenerPackage(true, JmsMessageListenerVariant.MATCHING_NAME_CONVENTION);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.INNER_CLASS);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.LAMBDA);
testJmsMessageListenerPackage(true, JmsMessageListenerVariant.NOT_MATCHING_NAME_CONVENTION);
}

@Test
public void testJmsMessageListenerPackage_applicationPackages() throws Exception {
doReturn(Arrays.asList("testapp")).when(config.getConfig(StacktraceConfiguration.class)).getApplicationPackages();

startAgent();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.javax.test;
package testapp;

import javax.jms.JMSException;
import javax.jms.Message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.javax.test;
package testapp;

import co.elastic.apm.agent.impl.Tracer;
import co.elastic.apm.agent.tracer.GlobalTracer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.jms.javax.test;
package testapp;


import co.elastic.apm.agent.impl.Tracer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import co.elastic.apm.agent.tracer.Tracer;
import co.elastic.apm.agent.tracer.configuration.CoreConfiguration;
import co.elastic.apm.agent.tracer.configuration.MessagingConfiguration;
import co.elastic.apm.agent.tracer.configuration.StacktraceConfiguration;
import net.bytebuddy.description.NamedElement;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.matcher.ElementMatcher;
Expand All @@ -33,20 +34,17 @@

import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass;
import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.isInAnyPackage;
import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.nameContains;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
import static net.bytebuddy.matcher.ElementMatchers.*;

public abstract class BaseJmsInstrumentation extends ElasticApmInstrumentation {

protected final MessagingConfiguration messagingConfiguration;
private final StacktraceConfiguration stacktraceConfiguration;

protected BaseJmsInstrumentation() {
Tracer tracer = GlobalTracer.get();
messagingConfiguration = tracer.getConfig(MessagingConfiguration.class);
stacktraceConfiguration = tracer.getConfig(StacktraceConfiguration.class);
}

@Override
Expand Down Expand Up @@ -84,6 +82,11 @@ public ElementMatcher<? super NamedElement> getMessageListenerTypeMatcherPreFilt
.or(nameContains("Message"))
.or(nameContains("Listener"));

Collection<String> applicationPackages = stacktraceConfiguration.getApplicationPackages();
if (!applicationPackages.isEmpty()) {
nameHeuristic = nameHeuristic.or(isInAnyPackage(applicationPackages, ElementMatchers.<NamedElement>none()));
}

Collection<String> listenerPackages = messagingConfiguration.getJmsListenerPackages();
if (listenerPackages.isEmpty()) {
// default heuristic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,11 @@ public class MessagingConfiguration extends ConfigurationOptionProvider {
.configurationCategory(MESSAGING_CATEGORY)
.description("Defines which packages contain JMS MessageListener implementations for instrumentation." +
"\n" +
"When set to a non-empty value, only the classes matching configuration will be instrumented.\n" +
"This configuration option helps to make MessageListener type matching faster and improve application startup performance."
"When empty (default), only the inner-classes or that have 'Listener' or 'Message' in their names are considered.\n"+
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
"\n" +
"This configuration option helps to make MessageListener type matching faster and improve application startup performance.\n" +
"\n" +
"Starting from version 1.43.0, the classes that are part of the 'application_packages' option are also included."
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
)
.dynamic(false)
.buildWithDefault(Collections.<String>emptyList());
Expand Down
10 changes: 8 additions & 2 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -2430,9 +2430,12 @@ Prepending an element with `(?-i)` makes the matching case sensitive.
==== `jms_listener_packages` (performance added[1.36.0])

Defines which packages contain JMS MessageListener implementations for instrumentation.
When set to a non-empty value, only the classes matching configuration will be instrumented.
When empty (default), only the inner-classes or that have 'Listener' or 'Message' in their names are considered.
jackshirazi marked this conversation as resolved.
Show resolved Hide resolved

This configuration option helps to make MessageListener type matching faster and improve application startup performance.

Starting from version 1.43.0, the classes that are part of the 'application_packages' option are also included.
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved




Expand Down Expand Up @@ -4514,8 +4517,11 @@ Example: `5ms`.
# ignore_message_queues=

# Defines which packages contain JMS MessageListener implementations for instrumentation.
# When set to a non-empty value, only the classes matching configuration will be instrumented.
# When empty (default), only the inner-classes or that have 'Listener' or 'Message' in their names are considered.
#
# This configuration option helps to make MessageListener type matching faster and improve application startup performance.
#
# Starting from version 1.43.0, the classes that are part of the 'application_packages' option are also included.
#
# This setting can not be changed at runtime. Changes require a restart of the application.
# Type: comma separated list
Expand Down
16 changes: 16 additions & 0 deletions docs/troubleshooting.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ Here, we will refer to the agent log file as `/tmp/agent.log`, but any other loc
. execute a few transactions which aren't properly captured by the agent
. copy the `/tmp/agent.log` file and send it back for investigation

[float]
[[trouble-shooting-matching-heuristics]]
=== Agent matching heuristics

The agent relies on heuristics to define which classes needs to be instrumented or not efficiently to prevent instrumentation
overhead.

Those heuristics are based on the packages and class names and are influenced by `application_packages` and
`jms_listener_packages` configurations. However, if instrumentation is not applied as expected or to enable creating configuration
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
without proper knowledge of the application internals, it could be relevant to disable those heuristics for investigation:

1. disable name heuristics by setting `enable_type_matching_name_pre_filtering=false` and enable the <<trouble-shooting-logging-procedure, agent logs>>.
2. restart the application, which will be slower than usual due to the extra overhead
3. analyze the agent logs to identify which classes/methods are instrumented by filtering lines with `Method match` string.
4. properly configure `application_packages` or `jms_listener_packages` with values that apply

[float]
[[trouble-shooting-debugging]]
=== Debugging
Expand Down
Loading