From 219d349bc39b19d9fe3807d6ed36a3d41ebfbb9a Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 9 Mar 2018 08:48:28 +0100 Subject: [PATCH] Polish messaging; fixes gh-893, fixes gh-894 --- .../TraceMessagingAutoConfiguration.java | 8 +- .../ITTracingChannelInterceptor.java | 236 ------------------ 2 files changed, 3 insertions(+), 241 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java index 3fe9c5464..e47fb17e0 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java @@ -19,6 +19,7 @@ package org.springframework.cloud.sleuth.instrument.messaging; import brave.Tracing; import brave.spring.rabbit.SpringRabbitTracing; import org.springframework.amqp.rabbit.config.SimpleRabbitListenerContainerFactory; +import org.springframework.amqp.rabbit.connection.Connection; import org.springframework.amqp.rabbit.connection.ConnectionFactory; import org.springframework.amqp.rabbit.core.RabbitTemplate; import org.springframework.beans.BeansException; @@ -56,12 +57,9 @@ public class TraceMessagingAutoConfiguration { @Bean @ConditionalOnMissingBean SpringRabbitTracing springRabbitTracing(Tracing tracing, - SleuthMessagingProperties properties, ConnectionFactory connectionFactory) { - String remoteServiceName = properties.getMessaging().getRemoteServiceName() + - (StringUtils.hasText(connectionFactory.getVirtualHost()) ? - "-" + connectionFactory.getVirtualHost() : ""); + SleuthMessagingProperties properties) { return SpringRabbitTracing.newBuilder(tracing) - .remoteServiceName(remoteServiceName) + .remoteServiceName(properties.getMessaging().getRemoteServiceName()) .build(); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/ITTracingChannelInterceptor.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/ITTracingChannelInterceptor.java index a7cc762b5..3444ed2e0 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/ITTracingChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/ITTracingChannelInterceptor.java @@ -111,242 +111,6 @@ public class ITTracingChannelInterceptor implements MessageHandler { .isNotNull(); } - //@Test - //public void parentSpanIncluded() { - // this.directChannel.send(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build()); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNotNull(); - // long traceId = Span - // .hexToId(this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class)); - // then(traceId).isEqualTo(10L); - // then(spanId).isNotEqualTo(20L); - // then(this.accumulator.getSpans()).hasSize(1); - //} - // - //@Test - //public void spanCreation() { - // this.directChannel.send(MessageBuilder.withPayload("hi").build()); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNotNull(); - // - // String traceId = this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class); - // then(traceId).isNotNull(); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //@Test - //public void shouldLogClientReceivedClientSentEventWhenTheMessageIsSentAndReceived() { - // this.directChannel.send(MessageBuilder.withPayload("hi").build()); - // - // then(this.accumulator.getSpans()).hasSize(1); - // then(this.accumulator.getSpans().get(0).logs()).extracting("event").contains(Span.CLIENT_SEND, - // Span.CLIENT_RECV); - //} - // - //@Test - //public void shouldLogServerReceivedServerSentEventWhenTheMessageIsPropagatedToTheNextListener() { - // this.directChannel.send(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.MESSAGE_SENT_FROM_CLIENT, true).build()); - // - // then(this.accumulator.getSpans()).hasSize(1); - // then(this.accumulator.getSpans().get(0).logs()).extracting("event").contains(Span.SERVER_RECV, - // Span.SERVER_SEND); - //} - // - //@Test - //public void headerCreation() { - // Span currentSpan = this.tracer.createSpan("http:testSendMessage", new AlwaysSampler()); - // this.directChannel.send(MessageBuilder.withPayload("hi").build()); - // this.tracer.close(currentSpan); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNotNull(); - // - // String traceId = this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class); - // then(traceId).isNotNull(); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //// TODO: Refactor to parametrized test together with sending messages via channel - //@Test - //public void headerCreationViaMessagingTemplate() { - // Span currentSpan = this.tracer.createSpan("http:testSendMessage", new AlwaysSampler()); - // this.messagingTemplate.send(MessageBuilder.withPayload("hi").build()); - // - // this.tracer.close(currentSpan); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNotNull(); - // - // String traceId = this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class); - // then(traceId).isNotNull(); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //@Test - //public void shouldCloseASpanWhenExceptionOccurred() { - // Span currentSpan = this.tracer.createSpan("http:testSendMessage", new AlwaysSampler()); - // Map errorHeaders = new HashMap<>(); - // errorHeaders.put("THROW_EXCEPTION", "TRUE"); - // - // try { - // this.messagingTemplate.send( - // MessageBuilder.withPayload("hi").copyHeaders(errorHeaders).build()); - // SleuthAssertions.fail("Exception should occur"); - // } - // catch (RuntimeException e) { - // } - // - // then(this.message).isNotNull(); - // this.tracer.close(currentSpan); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - // then(new ListOfSpans(this.accumulator.getSpans())) - // .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, - // "A terrible exception has occurred"); - //} - // - //@Test - //public void shouldNotTraceIgnoredChannel() { - // this.ignoredChannel.send(MessageBuilder.withPayload("hi").build()); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNull(); - // - // String traceId = this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class); - // then(traceId).isNull(); - // - // then(this.accumulator.getSpans()).isEmpty(); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //@Test - //public void downgrades128bitIdsByDroppingHighBits() { - // String hex128Bits = "463ac35c9f6413ad48485a3953bb6124"; - // String lower64Bits = "48485a3953bb6124"; - // this.directChannel.send(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, hex128Bits) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build()); - // then(this.message).isNotNull(); - // - // long traceId = Span.hexToId(this.message.getHeaders() - // .get(TraceMessageHeaders.TRACE_ID_NAME, String.class)); - // then(traceId).isEqualTo(Span.hexToId(lower64Bits)); - //} - // - //@Test - //public void shouldNotBreakWhenInvalidHeadersAreSent() { - // this.directChannel.send(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.PARENT_ID_NAME, "-") - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build()); - // - // then(this.message).isNotNull(); - // then(this.accumulator.getSpans()).isNotEmpty(); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //@Test - //public void shouldShortenTheNameWhenItsTooLarge() { - // this.directChannel.send(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.SPAN_NAME_NAME, bigName()) - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build()); - // - // then(this.message).isNotNull(); - // - // then(this.accumulator.getSpans()).isNotEmpty(); - // this.accumulator.getSpans().forEach(span1 -> then(span1.getName().length()).isLessThanOrEqualTo(50)); - // then(TestSpanContextHolder.getCurrentSpan()).isNull(); - //} - // - //private String bigName() { - // StringBuilder sb = new StringBuilder(); - // for (int i = 0; i < 60; i++) { - // sb.append("a"); - // } - // return sb.toString(); - //} - // - //@Test - //public void serializeMutableHeaders() throws Exception { - // Map headers = new HashMap<>(); - // headers.put("foo", "bar"); - // Message message = new GenericMessage<>("test", headers); - // ChannelInterceptor immutableMessageInterceptor = new ChannelInterceptorAdapter() { - // @Override - // public Message preSend(Message message, MessageChannel channel) { - // MessageHeaderAccessor headers = MessageHeaderAccessor.getMutableAccessor(message); - // return new GenericMessage(message.getPayload(), headers.toMessageHeaders()); - // } - // }; - // this.directChannel.addInterceptor(immutableMessageInterceptor); - // - // this.directChannel.send(message); - // - // Message output = (Message) SerializationUtils.deserialize(SerializationUtils.serialize(this.message)); - // then(output.getPayload()).isEqualTo("test"); - // then(output.getHeaders().get("foo")).isEqualTo("bar"); - // this.directChannel.removeInterceptor(immutableMessageInterceptor); - //} - // - //@Test - //public void workWithMessagingException() throws Exception { - // Message message = new GenericMessage<>(new MessagingException( - // MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build() - // )); - // - // this.directChannel.send(message); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(message.getPayload()).isEqualTo(this.message.getPayload()); - // then(spanId).isNotNull(); - // long traceId = Span - // .hexToId(this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class)); - // then(traceId).isEqualTo(10L); - // then(spanId).isNotEqualTo(20L); - // then(this.accumulator.getSpans()).hasSize(1); - //} - // - //@Test - //public void errorMessageHeadersRetained() { - // QueueChannel deadReplyChannel = new QueueChannel(); - // QueueChannel errorsReplyChannel = new QueueChannel(); - // Map errorChannelHeaders = new HashMap<>(); - // errorChannelHeaders.put(MessageHeaders.REPLY_CHANNEL, errorsReplyChannel); - // errorChannelHeaders.put(MessageHeaders.ERROR_CHANNEL, errorsReplyChannel); - // - // this.directChannel.send(new ErrorMessage( - // new MessagingException(MessageBuilder.withPayload("hi") - // .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) - // .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)) - // .setReplyChannel(deadReplyChannel) - // .setErrorChannel(deadReplyChannel) - // .build() ), - // errorChannelHeaders)); - // then(this.message).isNotNull(); - // - // String spanId = this.message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME, String.class); - // then(spanId).isNotNull(); - // long traceId = Span - // .hexToId(this.message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME, String.class)); - // then(traceId).isEqualTo(10L); - // then(spanId).isNotEqualTo(20L); - // then(this.accumulator.getSpans()).hasSize(1); - // then(this.message.getHeaders().getReplyChannel()).isSameAs(errorsReplyChannel); - // then(this.message.getHeaders().getErrorChannel()).isSameAs(errorsReplyChannel); - //} - @Configuration @EnableAutoConfiguration static class App { @Bean List spans() {