Added tracing for TopicConnection for JMS

fixes gh-1324
This commit is contained in:
Marcin Grzejszczak
2019-04-05 09:50:43 +02:00
parent 58b2115a1d
commit 1b493e602a
12 changed files with 188 additions and 81 deletions

View File

@@ -244,12 +244,6 @@
<version>3.8.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>net.jcip</groupId>
<artifactId>jcip-annotations</artifactId>
<version>1.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</dependencyManagement>

View File

@@ -336,11 +336,6 @@
<version>20.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>net.jcip</groupId>
<artifactId>jcip-annotations</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<profiles>

View File

@@ -272,8 +272,8 @@ public final class TracingChannelInterceptor extends ChannelInterceptorAdapter
headers.setImmutable();
if (message instanceof ErrorMessage) {
ErrorMessage errorMessage = (ErrorMessage) message;
return new ErrorMessage(errorMessage.getPayload(), headers.getMessageHeaders(),
errorMessage.getOriginalMessage());
return new ErrorMessage(errorMessage.getPayload(),
headers.getMessageHeaders(), errorMessage.getOriginalMessage());
}
return new GenericMessage<>(message.getPayload(), headers.getMessageHeaders());
}

View File

@@ -22,6 +22,8 @@ import javax.jms.JMSContext;
import javax.jms.JMSException;
import javax.jms.Message;
import javax.jms.MessageListener;
import javax.jms.TopicConnection;
import javax.jms.TopicConnectionFactory;
import javax.jms.XAConnection;
import javax.jms.XAConnectionFactory;
import javax.jms.XAJMSContext;
@@ -74,6 +76,10 @@ class TracingConnectionFactoryBeanPostProcessor implements BeanPostProcessor {
return new LazyXAConnectionFactory(this.beanFactory,
(XAConnectionFactory) bean);
}
else if (bean instanceof TopicConnectionFactory) {
return new LazyTopicConnectionFactory(this.beanFactory,
(TopicConnectionFactory) bean);
}
else if (bean instanceof ConnectionFactory) {
return new LazyConnectionFactory(this.beanFactory, (ConnectionFactory) bean);
}
@@ -135,6 +141,73 @@ class LazyXAConnectionFactory implements XAConnectionFactory {
}
class LazyTopicConnectionFactory implements TopicConnectionFactory {
private final BeanFactory beanFactory;
private final TopicConnectionFactory delegate;
private final LazyConnectionFactory factory;
private JmsTracing jmsTracing;
LazyTopicConnectionFactory(BeanFactory beanFactory, TopicConnectionFactory delegate) {
this.beanFactory = beanFactory;
this.delegate = delegate;
this.factory = new LazyConnectionFactory(beanFactory, delegate);
}
@Override
public TopicConnection createTopicConnection() throws JMSException {
return jmsTracing().topicConnection(this.delegate.createTopicConnection());
}
@Override
public TopicConnection createTopicConnection(String s, String s1)
throws JMSException {
return jmsTracing().topicConnection(this.delegate.createTopicConnection(s, s1));
}
@Override
public Connection createConnection() throws JMSException {
return this.factory.createConnection();
}
@Override
public Connection createConnection(String s, String s1) throws JMSException {
return this.factory.createConnection(s, s1);
}
@Override
public JMSContext createContext() {
return this.factory.createContext();
}
@Override
public JMSContext createContext(String s, String s1) {
return this.factory.createContext(s, s1);
}
@Override
public JMSContext createContext(String s, String s1, int i) {
return this.factory.createContext(s, s1, i);
}
@Override
public JMSContext createContext(int i) {
return this.factory.createContext(i);
}
private JmsTracing jmsTracing() {
if (this.jmsTracing != null) {
return this.jmsTracing;
}
this.jmsTracing = this.beanFactory.getBean(JmsTracing.class);
return this.jmsTracing;
}
}
class LazyConnectionFactory implements ConnectionFactory {
private final BeanFactory beanFactory;

View File

@@ -20,6 +20,8 @@ import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import javax.annotation.concurrent.NotThreadSafe;
import brave.Span;
import brave.Tracer;
import brave.sampler.Sampler;
@@ -50,6 +52,7 @@ import static reactor.core.publisher.Mono.just;
@SpringBootTest(classes = SleuthSpanCreatorAspectMonoTests.TestConfiguration.class)
@RunWith(SpringRunner.class)
@DirtiesContext(methodMode = BEFORE_METHOD)
@NotThreadSafe
public class SleuthSpanCreatorAspectMonoTests {
@Autowired

View File

@@ -69,15 +69,6 @@ public class HystrixAnnotationsIntegrationTests {
this.catcher.invokeLogicWrappedInHystrixCommand();
}
private void thenSpanInHystrixThreadIsContinued(final Span span) {
then(span).isNotNull();
Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> {
then(HystrixAnnotationsIntegrationTests.this.catcher).isNotNull();
then(span.context().traceId()).isEqualTo(
HystrixAnnotationsIntegrationTests.this.catcher.getTraceId());
});
}
private void thenSpanInHystrixThreadIsCreated() {
Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> {
then(HystrixAnnotationsIntegrationTests.this.catcher.getSpan()).isNotNull();
@@ -113,8 +104,10 @@ public class HystrixAnnotationsIntegrationTests {
@HystrixCommand
public void invokeLogicWrappedInHystrixCommand() {
System.out.println("FOOO");
this.spanCaughtFromHystrixThread = new AtomicReference<>(
this.tracing.tracer().currentSpan());
System.out.println("aksdhkasd: " + this.spanCaughtFromHystrixThread);
}
public Long getTraceId() {

View File

@@ -27,6 +27,8 @@ import javax.jms.Connection;
import javax.jms.ConnectionFactory;
import javax.jms.JMSException;
import javax.jms.MessageListener;
import javax.jms.TopicConnection;
import javax.jms.TopicConnectionFactory;
import javax.jms.XAConnection;
import javax.jms.XAConnectionFactory;
import javax.resource.spi.ResourceAdapter;
@@ -110,6 +112,22 @@ public class JmsTracingConfigurationTest {
}
}
static void checkTopicConnection(AssertableApplicationContext ctx)
throws JMSException {
// Not using try-with-resources as that doesn't exist in JMS 1.1
TopicConnection con = ctx.getBean(TopicConnectionFactory.class)
.createTopicConnection();
try {
con.setExceptionListener(exception -> {
});
assertThat(con.getExceptionListener().getClass().getName())
.startsWith("brave.jms.TracingExceptionListener");
}
finally {
con.close();
}
}
@Test
public void tracesConnectionFactory() {
this.contextRunner.run(JmsTracingConfigurationTest::checkConnection);
@@ -124,6 +142,15 @@ public class JmsTracingConfigurationTest {
});
}
@Test
public void tracesTopicConnectionFactories() {
this.contextRunner.withUserConfiguration(XAConfiguration.class).run(ctx -> {
clearSpans(ctx);
checkConnection(ctx);
checkTopicConnection(ctx);
});
}
@Test
public void tracesListener_jmsMessageListener() {
this.contextRunner.withUserConfiguration(SimpleJmsListenerConfiguration.class)

View File

@@ -296,8 +296,8 @@ public class TracingChannelInterceptorTest {
this.message = this.channel.receive();
assertThat(this.message).isNotNull();
assertThat(this.message)
.isInstanceOfSatisfying(ErrorMessage.class, errorMessage -> {
assertThat(this.message).isInstanceOfSatisfying(ErrorMessage.class,
errorMessage -> {
assertThat(errorMessage.getOriginalMessage())
.isSameAs(originalMessage);
assertThat(errorMessage.getHeaders().get("header"))

View File

@@ -19,10 +19,11 @@ package org.springframework.cloud.sleuth.instrument.scheduling;
import java.util.AbstractMap;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.concurrent.NotThreadSafe;
import brave.Span;
import brave.Tracing;
import brave.sampler.Sampler;
import net.jcip.annotations.NotThreadSafe;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.junit.Before;
@@ -185,11 +186,9 @@ class TestBeanWithScheduledMethod {
this.tracing = tracing;
}
@Scheduled(fixedDelay = 1L)
@Scheduled(fixedDelay = 1000L)
public void scheduledMethod() {
log.info("Running the scheduled method");
this.span = this.tracing.tracer().currentSpan();
log.info("Stored the span " + this.span + " as current span");
this.executed.set(true);
}
@@ -258,7 +257,7 @@ class TestBeanWithScheduledMethodToBeIgnored {
this.tracing = tracing;
}
@Scheduled(fixedDelay = 1L)
@Scheduled(fixedDelay = 1000L)
public void scheduledMethodToIgnore() {
this.span = this.tracing.tracer().currentSpan();
this.executed.set(true);

View File

@@ -53,11 +53,13 @@ import static org.assertj.core.api.BDDAssertions.then;
public class SkipPatternProviderConfigTest {
private final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(DispatcherServletAutoConfiguration.class,
InfoEndpointAutoConfiguration.class, HealthIndicatorAutoConfiguration.class,
HealthEndpointAutoConfiguration.class, EndpointAutoConfiguration.class,
WebEndpointAutoConfiguration.class, TraceAutoConfiguration.class,
TraceWebAutoConfiguration.class));
.withConfiguration(AutoConfigurations.of(
DispatcherServletAutoConfiguration.class,
InfoEndpointAutoConfiguration.class,
HealthIndicatorAutoConfiguration.class,
HealthEndpointAutoConfiguration.class,
EndpointAutoConfiguration.class, WebEndpointAutoConfiguration.class,
TraceAutoConfiguration.class, TraceWebAutoConfiguration.class));
@Test
public void should_pick_skip_pattern_from_sleuth_properties() throws Exception {
@@ -93,12 +95,13 @@ public class SkipPatternProviderConfigTest {
@Test
public void should_return_management_context_with_context_path() throws Exception {
contextRunner
.withConfiguration(UserConfigurations.of(
ManagementContextAutoConfiguration.class, ServerPropertiesConfig.class))
.withConfiguration(
UserConfigurations.of(ManagementContextAutoConfiguration.class,
ServerPropertiesConfig.class))
.withPropertyValues("management.server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/actuator/(health|health/.*|info|info/.*)", "foo.*",
"/actuator/(health|health/.*|info|info/.*)", "foo.*",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@@ -116,94 +119,113 @@ public class SkipPatternProviderConfigTest {
@Test
public void should_return_endpoints_without_context_path() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/actuator/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/actuator/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_context_path() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("server.servlet.context-path=foo")
.run(context -> {
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("server.servlet.context-path=foo").run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"foo/actuator/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"foo/actuator/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_without_context_path_and_base_path_set_to_root() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.endpoints.web.base-path=/")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_context_path_and_base_path_set_to_root() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.endpoints.web.base-path=/",
"server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"foo/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"foo/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_context_path_and_base_path_set_to_root_different_port() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.endpoints.web.base-path=/",
"management.server.port=0", "server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_actuator_context_path_only() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.endpoints.web.base-path=/mgt",
"server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"foo/mgt/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"foo/mgt/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_actuator_default_context_path_different_port() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.server.port=0", "server.servlet.context-path=foo")
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.server.port=0",
"server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/actuator/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/actuator/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_actuator_context_path_only_different_port() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.endpoints.web.base-path=/mgt",
"management.server.port=0", "server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/mgt/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/mgt/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@Test
public void should_return_endpoints_with_context_path_different_port() {
contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.server.port=0", "server.servlet.context-path=foo")
contextRunner
.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class))
.withPropertyValues("management.server.port=0",
"server.servlet.context-path=foo")
.run(context -> {
then(extractAllPatterns(context)).containsExactlyInAnyOrder(
"/actuator/(health|health/.*|info|info/.*)", SleuthWebProperties.DEFAULT_SKIP_PATTERN);
"/actuator/(health|health/.*|info|info/.*)",
SleuthWebProperties.DEFAULT_SKIP_PATTERN);
});
}
@@ -224,32 +246,29 @@ public class SkipPatternProviderConfigTest {
private SingleSkipPattern bar() {
return () -> Optional.of(Pattern.compile("bar"));
}
/**
* Extracts the patterns from pattern provider
* Extracts the patterns from pattern provider
*/
private String extractPattern(ApplicationContext context) {
SkipPatternProvider skipPatternProvider = context.getBean(SkipPatternProvider.class);
SkipPatternProvider skipPatternProvider = context
.getBean(SkipPatternProvider.class);
return skipPatternProvider.skipPattern().pattern();
}
/**
* Extracts all single patterns
*/
* Extracts all single patterns
*/
private Collection<String> extractAllPatterns(ApplicationContext context) {
return context
.getBeansOfType(SingleSkipPattern.class)
.values()
.stream()
.map(SingleSkipPattern::skipPattern)
.filter(Optional::isPresent)
.map(Optional::get)
.map(Pattern::pattern)
.collect(Collectors.toList());
return context.getBeansOfType(SingleSkipPattern.class).values().stream()
.map(SingleSkipPattern::skipPattern).filter(Optional::isPresent)
.map(Optional::get).map(Pattern::pattern).collect(Collectors.toList());
}
@Configuration
@EnableConfigurationProperties(ServerProperties.class)
static class ServerPropertiesConfig {
}
}

View File

@@ -19,6 +19,8 @@ package org.springframework.cloud.sleuth.instrument.web;
import java.util.List;
import java.util.stream.Collectors;
import javax.annotation.concurrent.NotThreadSafe;
import brave.Span;
import brave.Tracer;
import brave.sampler.Sampler;
@@ -64,6 +66,7 @@ import org.springframework.web.reactive.function.server.ServerResponse;
import static org.assertj.core.api.BDDAssertions.then;
@NotThreadSafe
public class TraceWebFluxTests {
public static final String EXPECTED_TRACE_ID = "b919095138aa4c6e";

View File

@@ -44,13 +44,14 @@ import static org.assertj.core.api.BDDAssertions.then;
@RunWith(SpringRunner.class)
@SpringBootTest(classes = ZipkinDiscoveryClientTests.Config.class, properties = {
"spring.zipkin.baseUrl=https://zipkin/", "spring.zipkin.sender.type=web" // override
// default
// priority
// which
// picks
// rabbit
// due to
// classpath
// default
// priority
// which
// picks
// rabbit
// due
// to
// classpath
})
public class ZipkinDiscoveryClientTests {