diff --git a/CHANGELOG.md b/CHANGELOG.md index 92080e36bb..863b1619ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,12 @@ -# next (1.7.0) +# next (1.8.0) + +## Features + * Add support for Spring's JMS flavor - instrumenting `org.springframework.jms.listener.SessionAwareMessageListener` + +## Bug Fixes + * Some JMS Consumers and Producers are filtered due to class name filtering in instrumentation matching + +# 1.7.0 ## Features * Added the `trace_methods_duration_threshold` config option. When using the `trace_methods` config option with wild cards, this diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index b56f93f762..5556055bbc 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -199,6 +199,8 @@ public boolean asChildOf(String traceParentHeader) { } catch (IllegalArgumentException e) { logger.warn(e.getMessage()); return false; + } finally { + onMutation(); } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/TraceContextTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/TraceContextTest.java index 2d621f032e..864ab53f09 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/TraceContextTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/transaction/TraceContextTest.java @@ -96,6 +96,14 @@ void testResetState() { assertThat(traceContext.getIncomingTraceParentHeader()).isEqualTo("00-00000000000000000000000000000000-0000000000000000-00"); } + @Test + void testResetOutgoingHeader() { + final TraceContext traceContext = TraceContext.with64BitId(mock(ElasticApmTracer.class)); + String traceParentHeader = traceContext.getOutgoingTraceParentHeader().toString(); + traceContext.asChildOf("00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-00"); + assertThat(traceContext.getOutgoingTraceParentHeader().toString()).isNotEqualTo(traceParentHeader); + } + @Test void testRandomValue() { final TraceContext traceContext = TraceContext.with64BitId(mock(ElasticApmTracer.class)); diff --git a/apm-agent-plugins/apm-jms-plugin/pom.xml b/apm-agent-plugins/apm-jms-plugin/pom.xml index 025333abdd..7a14546d8f 100644 --- a/apm-agent-plugins/apm-jms-plugin/pom.xml +++ b/apm-agent-plugins/apm-jms-plugin/pom.xml @@ -9,6 +9,9 @@ ${project.basedir}/../.. + 5.1.8.RELEASE + 5.15.9 + 2.8.1 apm-jms-plugin @@ -24,19 +27,31 @@ org.apache.activemq activemq-broker - 5.15.9 + ${activemq.version} test org.apache.activemq artemis-jms-client - 2.8.1 + ${artemis.version} test org.apache.activemq artemis-jms-server - 2.8.1 + ${artemis.version} + test + + + org.springframework + spring-jms + ${spring-framework.version} + test + + + org.springframework + spring-context + ${spring-framework.version} test diff --git a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageConsumerInstrumentation.java b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageConsumerInstrumentation.java index 45fd335d32..8ca5b75e91 100644 --- a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageConsumerInstrumentation.java +++ b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageConsumerInstrumentation.java @@ -69,7 +69,10 @@ public abstract class JmsMessageConsumerInstrumentation extends BaseJmsInstrumen @Override public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("Message").or(nameContains("Consumer")); + return nameContains("Message") + .or(nameContains("Consumer")) + .or(nameContains("Receiver")) + .or(nameContains("Subscriber")); } @Override diff --git a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageListenerInstrumentation.java b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageListenerInstrumentation.java index be103949af..15207758bd 100644 --- a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageListenerInstrumentation.java +++ b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageListenerInstrumentation.java @@ -63,14 +63,17 @@ public JmsMessageListenerInstrumentation(ElasticApmTracer tracer) { @Override public ElementMatcher getTypeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))); + return not(isInterface()) + .and( + hasSuperType(named("javax.jms.MessageListener")) + .or(hasSuperType(named("org.springframework.jms.listener.SessionAwareMessageListener"))) + ); } @Override public ElementMatcher getMethodMatcher() { return named("onMessage") - .and(takesArguments(1)) - .and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()); + .and(takesArgument(0, hasSuperType(named("javax.jms.Message")))).and(isPublic()); } @Override diff --git a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageProducerInstrumentation.java b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageProducerInstrumentation.java index e99802c7ac..6a953e7d3d 100644 --- a/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageProducerInstrumentation.java +++ b/apm-agent-plugins/apm-jms-plugin/src/main/java/co/elastic/apm/agent/jms/JmsMessageProducerInstrumentation.java @@ -68,7 +68,10 @@ public abstract class JmsMessageProducerInstrumentation extends BaseJmsInstrumen @Override public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("Message").or(nameContains("Producer")); + return nameContains("Message") + .or(nameContains("Producer")) + .or(nameContains("Sender")) + .or(nameContains("Publisher")); } @Override diff --git a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java new file mode 100644 index 0000000000..d4e5e3563c --- /dev/null +++ b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java @@ -0,0 +1,134 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.jms.spring; + +import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.transaction.Id; +import co.elastic.apm.agent.impl.transaction.Span; +import co.elastic.apm.agent.impl.transaction.TraceContext; +import co.elastic.apm.agent.impl.transaction.Transaction; +import org.apache.activemq.ActiveMQConnectionFactory; +import org.apache.activemq.command.ActiveMQMapMessage; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +import javax.jms.Connection; +import javax.jms.ConnectionFactory; +import javax.jms.JMSException; +import javax.jms.MapMessage; +import javax.jms.MessageProducer; +import javax.jms.Queue; +import javax.jms.Session; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +public class SpringJmsTest extends AbstractInstrumentationTest { + + private static final String SPRING_TEST_QUEUE = "Spring-Test-Queue"; + static final BlockingQueue resultQueue = new ArrayBlockingQueue<>(5); + + private static Connection connection; + private static ClassPathXmlApplicationContext ctx; + + @BeforeClass + public static void setup() throws JMSException { + ConnectionFactory connectionFactory = new ActiveMQConnectionFactory("vm://localhost?broker.persistent=false"); + connection = connectionFactory.createConnection(); + connection.start(); + ctx = new ClassPathXmlApplicationContext("app-context.xml"); + } + + @AfterClass + public static void teardown() throws JMSException { + ctx.close(); + connection.stop(); + } + + @Test + public void testSendListenSpringQueue() throws JMSException, InterruptedException { + reporter.reset(); + try (Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE)) { + + Transaction transaction = tracer.startTransaction(TraceContext.asRoot(), null, null).activate(); + transaction.setName("JMS-Spring-Test Transaction"); + transaction.withType("request"); + transaction.withResult("success"); + + final String key1 = "key1"; + final String key2 = "key2"; + final String value1 = UUID.randomUUID().toString(); + final String value2 = UUID.randomUUID().toString(); + MapMessage mapMessage = new ActiveMQMapMessage(); + mapMessage.setString(key1, value1); + mapMessage.setString(key2, value2); + + Queue queue = session.createQueue(SPRING_TEST_QUEUE); + MessageProducer producer = session.createProducer(queue); + producer.send(mapMessage); + + // Let the onMessage instrumentation end the transaction then end the base transaction + Thread.sleep(500); + transaction.deactivate().end(); + + Map result = resultQueue.poll(1, TimeUnit.SECONDS); + + assertThat(result).isNotNull(); + assertThat(result.size()).isEqualTo(2); + assertThat(result.get(key1).toString()).isEqualTo(value1); + assertThat(result.get(key2).toString()).isEqualTo(value2); + + List transactions = reporter.getTransactions(); + // The way the Spring framework works is polling through a standard JMS receive API in order to get the + // message, with which it then invokes the SpringMapMessageListener.onMessage() implementation, so we expect + // two JMS receive transactions (one for the receive and one for the onMessage), both with same parent and traceId + assertThat(transactions).hasSize(3); + Transaction baseTransaction = transactions.get(2); + Id traceId = baseTransaction.getTraceContext().getTraceId(); + + List spans = reporter.getSpans(); + assertThat(spans).hasSize(1); + Span sendSpan = spans.get(0); + assertThat(sendSpan.getName().toString()).isEqualTo("JMS SEND to queue " + SPRING_TEST_QUEUE); + assertThat(sendSpan.getTraceContext().getTraceId()).isEqualTo(traceId); + + Transaction receiveTransaction = transactions.get(0); + verifyReceiveTransaction(traceId, sendSpan, receiveTransaction); + } + } + + private void verifyReceiveTransaction(Id traceId, Span sendSpan, Transaction receiveTransaction) { + assertThat(receiveTransaction.getName().toString()).isEqualTo("JMS RECEIVE from queue " + SPRING_TEST_QUEUE); + assertThat(receiveTransaction.getTraceContext().getTraceId()).isEqualTo(traceId); + assertThat(receiveTransaction.getTraceContext().getParentId()).isEqualTo(sendSpan.getTraceContext().getId()); + } +} diff --git a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringMapMessageListener.java b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringMapMessageListener.java new file mode 100644 index 0000000000..0599d8d7fc --- /dev/null +++ b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringMapMessageListener.java @@ -0,0 +1,46 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.jms.spring; + +import org.apache.activemq.command.ActiveMQMapMessage; +import org.springframework.jms.listener.SessionAwareMessageListener; +import org.springframework.stereotype.Service; + +import javax.jms.JMSException; +import javax.jms.MapMessage; +import javax.jms.Session; +import java.util.Map; + +@Service +public class SpringMapMessageListener implements SessionAwareMessageListener { + + @Override + public void onMessage(MapMessage mapMessage, Session session) throws JMSException { + Map map = ((ActiveMQMapMessage) mapMessage).getContentMap(); + System.out.println("Received map message: "); + map.forEach((key, value) -> System.out.println(" " + key + " --> " + value)); + SpringJmsTest.resultQueue.offer(map); + } +} diff --git a/apm-agent-plugins/apm-jms-plugin/src/test/resources/app-context.xml b/apm-agent-plugins/apm-jms-plugin/src/test/resources/app-context.xml new file mode 100644 index 0000000000..2b0492127b --- /dev/null +++ b/apm-agent-plugins/apm-jms-plugin/src/test/resources/app-context.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +