Uses B3 single for non-remote spans (2.2.x) (#1655)

This makes sure there cannot be JMS problems related to use of
hyphenated headers unless someone overrides the `Propagation.Factory`.

To avoid this, we set non-remote spans to B3 single format as done on
the 3.x branch here:
https://github.com/spring-cloud/spring-cloud-sleuth/pull/1607/files#diff-db43b7e91bd69d063333c20b947c902bR83-R84
This commit is contained in:
Adrian Cole
2020-05-26 14:42:27 +08:00
committed by GitHub
parent 473549f000
commit a6573bc2a5
4 changed files with 31 additions and 46 deletions

View File

@@ -25,6 +25,7 @@ import brave.baggage.BaggagePropagation;
import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
import brave.baggage.BaggagePropagationCustomizer;
import brave.propagation.B3Propagation;
import brave.propagation.B3Propagation.Format;
import brave.propagation.ExtraFieldCustomizer;
import brave.propagation.ExtraFieldPropagation;
import brave.propagation.Propagation;
@@ -57,6 +58,11 @@ class TraceBaggageConfiguration {
static final String BAGGAGE_KEYS = "spring.sleuth.baggage-keys";
static final String PROPAGATION_KEYS = "spring.sleuth.propagation-keys";
// Note: Versions <2.2.3 use injectFormat(MULTI) for non-remote (ex spring-messaging)
// See #1643
static final Propagation.Factory B3_FACTORY = B3Propagation.newFactoryBuilder()
.injectFormat(Format.SINGLE_NO_PARENT).build();
// These List<String> beans allow us to get deprecated property values, regardless of
// if they were comma or yaml encoded. This keeps them out of SleuthBaggageProperties
@@ -82,14 +88,15 @@ class TraceBaggageConfiguration {
* To override the underlying context format, override this bean and set the delegate
* to what you need. {@link BaggagePropagation.FactoryBuilder} will unwrap itself if
* no fields are configured.
*
* <p>
* This will use {@link Format#SINGLE_NO_PARENT} for non-remote spans, such as for
* messaging. Note: it will still parse incoming multi-header spans.
*/
@Bean
@ConditionalOnMissingBean
BaggagePropagation.FactoryBuilder baggagePropagationFactoryBuilder() {
// Default for spring-messaging is on 2.2.x is MULTI, though 3.x it is
// SINGLE_NO_PARENT spring-cloud/spring-cloud-sleuth#1607
return BaggagePropagation.newFactoryBuilder(B3Propagation.newFactoryBuilder()
.injectFormat(B3Propagation.Format.MULTI).build());
return BaggagePropagation.newFactoryBuilder(B3_FACTORY);
}
Propagation.Factory sleuthPropagation(
@@ -105,8 +112,7 @@ class TraceBaggageConfiguration {
factoryBuilder = extraFieldPropagationFactoryBuilder;
}
else {
factoryBuilder = ExtraFieldPropagation
.newFactoryBuilder(B3Propagation.FACTORY);
factoryBuilder = ExtraFieldPropagation.newFactoryBuilder(B3_FACTORY);
}
if (!baggageKeys.isEmpty()) {
factoryBuilder

View File

@@ -16,8 +16,6 @@
package org.springframework.cloud.sleuth.autoconfig;
import brave.propagation.B3Propagation;
import brave.propagation.B3Propagation.Format;
import brave.propagation.B3SinglePropagation;
import brave.propagation.ExtraFieldPropagation;
import brave.propagation.Propagation;
@@ -34,11 +32,6 @@ import org.springframework.context.support.GenericApplicationContext;
public class TraceAutoConfigurationPropagationCustomizationTests {
// Default for spring-messaging is on 2.2.x is MULTI, though 3.x it is
// SINGLE_NO_PARENT spring-cloud/spring-cloud-sleuth#1607
Propagation.Factory defaultB3Propagation = B3Propagation.newFactoryBuilder()
.injectFormat(Format.MULTI).build();
private final ApplicationContextRunner contextRunner = new ApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(TraceAutoConfiguration.class));
@@ -46,7 +39,7 @@ public class TraceAutoConfigurationPropagationCustomizationTests {
public void stillCreatesDefault() {
this.contextRunner.run((context) -> {
BDDAssertions.then(context.getBean(Propagation.Factory.class))
.isEqualTo(defaultB3Propagation);
.isEqualTo(TraceBaggageConfiguration.B3_FACTORY);
});
}
@@ -56,7 +49,7 @@ public class TraceAutoConfigurationPropagationCustomizationTests {
.run((context) -> {
BDDAssertions.then(context.getBean(Propagation.Factory.class))
.hasFieldOrPropertyWithValue("delegate",
B3Propagation.FACTORY);
TraceBaggageConfiguration.B3_FACTORY);
});
}
@@ -65,7 +58,7 @@ public class TraceAutoConfigurationPropagationCustomizationTests {
this.contextRunner.withPropertyValues("spring.application.name=")
.run((context) -> {
BDDAssertions.then(context.getBean(Propagation.Factory.class))
.isEqualTo(B3Propagation.FACTORY);
.isEqualTo(TraceBaggageConfiguration.B3_FACTORY);
});
}

View File

@@ -20,6 +20,8 @@ import brave.Span;
import brave.Tracer;
import brave.Tracing;
import brave.handler.SpanHandler;
import brave.propagation.B3SingleFormat;
import brave.propagation.TraceContext;
import brave.sampler.Sampler;
import brave.test.TestSpanHandler;
import org.junit.After;
@@ -30,7 +32,6 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.sleuth.instrument.util.SpanUtil;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.integration.channel.QueueChannel;
@@ -68,7 +69,7 @@ public class TraceContextPropagationChannelInterceptorTests {
@Test
public void testSpanPropagation() {
Span span = this.tracing.tracer().nextSpan().name("http:testSendMessage").start();
String expectedSpanId = SpanUtil.idToHex(span.context().spanId());
String expectedSpanId = span.context().spanIdString();
try (Tracer.SpanInScope ws = this.tracing.tracer().withSpanInScope(span)) {
this.channel.send(MessageBuilder.withPayload("hi").build());
@@ -84,19 +85,12 @@ public class TraceContextPropagationChannelInterceptorTests {
Message<?> message = this.channel.receive(0);
assertThat(message).as("message was null").isNotNull();
String spanId = message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME,
String.class);
assertThat(spanId).as("spanId was equal to parent's id")
String b3 = message.getHeaders().get("b3", String.class);
// Trace and Span IDs are implicitly checked
TraceContext extracted = B3SingleFormat.parseB3SingleFormat(b3).context();
assertThat(extracted.spanIdString()).as("spanId was equal to parent's id")
.isNotEqualTo(expectedSpanId);
String traceId = message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME,
String.class);
assertThat(traceId).as("traceId was null").isNotNull();
String parentId = message.getHeaders().get(TraceMessageHeaders.PARENT_ID_NAME,
String.class);
assertThat(parentId).as("parentId was not equal to parent's id")
.isEqualTo(this.spans.get(0).id());
}
@Configuration

View File

@@ -20,6 +20,8 @@ import brave.Span;
import brave.Tracer;
import brave.Tracing;
import brave.handler.SpanHandler;
import brave.propagation.B3SingleFormat;
import brave.propagation.TraceContext;
import brave.sampler.Sampler;
import brave.test.TestSpanHandler;
import org.junit.After;
@@ -30,7 +32,6 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.sleuth.instrument.util.SpanUtil;
import org.springframework.cloud.stream.binder.test.OutputDestination;
import org.springframework.cloud.stream.binder.test.TestChannelBinderConfiguration;
import org.springframework.cloud.stream.function.StreamBridge;
@@ -71,7 +72,7 @@ public class TraceStreamChannelInterceptorTests {
@Test
public void testSpanPropagationViaBridge() {
Span span = this.tracing.tracer().nextSpan().name("http:testSendMessage").start();
String expectedSpanId = SpanUtil.idToHex(span.context().spanId());
String expectedSpanId = span.context().spanIdString();
try (Tracer.SpanInScope ws = this.tracing.tracer().withSpanInScope(span)) {
this.streamBridge.send("testSupplier-out-0", "hi");
@@ -87,21 +88,12 @@ public class TraceStreamChannelInterceptorTests {
Message<?> message = this.channel.receive(0);
assertThat(message).as("message was null").isNotNull();
String spanId = message.getHeaders().get(TraceMessageHeaders.SPAN_ID_NAME,
String.class);
assertThat(spanId).as("spanId was equal to parent's id")
String b3 = message.getHeaders().get("b3", String.class);
// Trace and Span IDs are implicitly checked
TraceContext extracted = B3SingleFormat.parseB3SingleFormat(b3).context();
assertThat(extracted.spanIdString()).as("spanId was equal to parent's id")
.isNotEqualTo(expectedSpanId);
String traceId = message.getHeaders().get(TraceMessageHeaders.TRACE_ID_NAME,
String.class);
assertThat(traceId).as("traceId was null").isNotNull();
String parentId = message.getHeaders().get(TraceMessageHeaders.PARENT_ID_NAME,
String.class);
// [0] - producer
// [1] - http:testsendmessage
assertThat(parentId).as("parentId was not equal to parent's id")
.isEqualTo(this.spans.get(1).id());
}
@Configuration