Notes on Sleuth to Brave migration

sleuth_brave.md

Notes

TODO:

• Messaging
• Baggage

Discuss what do we do with the TraceKeys in callables / runnables

Done:

• Web Server
  • Async servlet
  • Reactive servlet
• Reactor
• RxJava
• Web Clients
  • RestTemplate
  • Feign
• Slf4j
  • with Brave in place it will also work for any other implementation (e.g. log4j)
• Async
  • Runnable / Callable
  • TraceExecutors etc.
  • @Async
• Scheduling
• Hystrix
• Spans via annotations
• Web Clients
  • Reactive WebClient
• Zuul

Migrations

Core

With Brave instrumentation there are 2 different lifecycles.

• Span creation and stopping and span reporting.
• Span hooking to current context

When brave.Span.finish() is called the span gets stopped and reported. In order to hook in the span to current context you need to call the try-with-resources clause via withSpanInScope try (Tracer.SpanInScope ws = this.tracing.tracer().withSpanInScope(this.span.start())) { // do sth with the span } finally { this.span.finish(); }

Removed features

• SpanLogger
  • Name pattern of Span Logger will not be applicable.
• Sampler
  • TODO: describe the new sampling mechanism in the documentation
• Metrics
  • we delegate to ReporterMetrics from Zipkin
• SleuthProperties props:
  • supportsJoin - will come from Brave

Sleuth's Tracer to brave.Tracing

Before

import org.springframework.cloud.sleuth.Tracer;

Tracer tracer;

After

import brave.Tracing;

Tracing tracing;

Brave's spans are not started by default

In Sleuth whenever you've created a span it got attached to the current context and started. In Brave you have to manage both manually. When the span is created, it's not started. You need to explicitly call start() on the span to make it reportable to Zipkin. The tracing context will be passed but the span will not be sent to Zipkin if you don't start it.

TODO: Double check this ^^

Child span creation

Before

Span child = tracer.createSpan("name");

After

// tracing is brave.Tracing injected instead of Sleuth's Tracer
brave.Span span = tracing.tracer().nextSpan().name("name");

Span closing

Before

Span child = tracer.createSpan("name");
try {
  // do sth
} finally {
  tracer.close(child);
}

After

// tracing is brave.Tracing injected instead of Sleuth's Tracer
brave.Span span = this.tracing.tracer().nextSpan().name("name");
try {
  // do sth
} finally {
  span.finish();
}

Span tagging

Before

tracer.addTag("foo", "bar");

After

// tracing is brave.Tracing injected instead of Sleuth's Tracer
this.tracing.tracer().currentSpan().tag("foo", "bar");

Baggage needs to be whitelisted

In Sleuth we used to create headers that had the baggage prefix

ArayListSpanAccumulator renamed to ArrayListSpanReporter

Before

org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator

After

org.springframework.cloud.sleuth.util.ArrayListSpanReporter

Percentage -> Probability

org.springframework.cloud.brave.sampler.SamplerProperties#percentage renamed to org.springframework.cloud.brave.sampler.SamplerProperties#probability

and

org.springframework.cloud.sleuth.sampler.PercentageBasedSampler renamed to org.springframework.cloud.sleuth.sampler.ProbabilityBasedSampler

Related to spring-cloud/spring-cloud-sleuth#397

Async

TraceRunnable and TraceCallable moved to instrument.async

Before

org.springframework.cloud.sleuth.TraceRunnable
org.springframework.cloud.sleuth.TraceCallable

After

org.springframework.cloud.sleuth.instrument.async.TraceRunnable
org.springframework.cloud.sleuth.instrument.async.TraceCallable

TraceableExecutorService has the constructor with BeanFactory remaining

Before

public TraceableExecutorService(final ExecutorService delegate, final Tracer tracer,
			TraceKeys traceKeys, SpanNamer spanNamer) {
    //...
	}

	public TraceableExecutorService(BeanFactory beanFactory, final ExecutorService delegate) {
    //...
	}

	public TraceableExecutorService(final ExecutorService delegate, final Tracer tracer,
			TraceKeys traceKeys, SpanNamer spanNamer, String spanName) {
		//...
	}

After

public TraceableExecutorService(BeanFactory beanFactory, final ExecutorService delegate) {
		//...
	}


public TraceableExecutorService(BeanFactory beanFactory, final ExecutorService delegate, String spanName) {
		//...
	}

TraceableScheduledExecutorService changed constructor

Before

public TraceableScheduledExecutorService(ScheduledExecutorService delegate,
			Tracer tracer, TraceKeys traceKeys, SpanNamer spanNamer) {
		super(delegate, tracer, traceKeys, spanNamer);
	}

After

public TraceableScheduledExecutorService(BeanFactory beanFactory, final ExecutorService delegate) {
		super(beanFactory, delegate);
	}

TraceAsyncAspect changed constructor

Before

@Deprecated
	public TraceAsyncAspect(Tracer tracer, TraceKeys traceKeys, BeanFactory beanFactory) {
		this.tracer = tracer;
		this.traceKeys = traceKeys;
		this.beanFactory = beanFactory;
	}

	public TraceAsyncAspect(Tracer tracer, TraceKeys traceKeys, SpanNamer spanNamer) {
		this.tracer = tracer;
		this.traceKeys = traceKeys;
		this.spanNamer = spanNamer;
		this.beanFactory = null;
	}

After

public TraceAsyncAspect(Tracing tracing, SpanNamer spanNamer, TraceKeys traceKeys) {
		//
	}

TraceAsyncAspect overrides the name of the current span

In Sleuth we were able to rename the async span coming from a TraceRunnable. In Brave you can't access the span's name when it's in progress of being created. That's why we will always rename the span.

TraceAsyncListenableTaskExecutor moved from web.client to async package

Before

org.springframework.cloud.sleuth.instrument.web.client.TraceAsyncListenableTaskExecutor

After

org.springframework.cloud.sleuth.instrument.async.TraceAsyncListenableTaskExecutor

Removed features

It seems that we have overengineered the async trace context passing. It seems that with the async template's interceptor with a wrapper around the ListenableFutureCallback we don't need to wrap the connection factories anymore.

TODO: Double check that this is true ^^

Removed classes :

• TraceAsyncClientHttpRequestFactoryWrapper - replaced by simple interceptor org.springframework.cloud.brave.instrument.web.client.AsyncTracingClientHttpRequestInterceptor
• TraceRestTemplateInterceptor - replaced by brave.spring.web.TracingClientHttpRequestInterceptor
• LocalComponentTraceCallable - replaced by TraceCallable
• LocalComponentTraceRunnable - replaced by TraceRunnable
• SpanContinuingTraceCallable - replaced by TraceCallable
• SpanContinuingTraceRunnable - replaced by TraceRunnable

Web clients (RestTemplate, WebClient, AsyncRestTemplate)

AsyncRestTemplate no longer a bean

We've changed the way AsyncRestTemplate is instrumented. No longer do we instrument in a way that automatically registers a AsyncRestTemplate bean. That's why you need to provide your own bean.

Before

no AsyncRestTemplate bean was required

After

@Bean
AsyncRestTemplate myAsyncRestTemplate() {
return new AsyncRestTemplate();
}

Removed classes :

• TraceAsyncRestTemplate - replaced by simple interceptor org.springframework.cloud.brave.instrument.web.client.AsyncTracingClientHttpRequestInterceptor

RestTemplate interceptors removed

We will use those from Brave.

Before

org.springframework.cloud.sleuth.instrument.web.client.TraceRestTemplateInterceptor

After

brave.spring.web.TracingClientHttpRequestInterceptor

Brave RestTemplate interceptor name and tag spans differently

When a request was sent via RestTemplate it will name the span with the method name e.g. GET. If you want to have the previous way of naming spans and the default span tags you have turn on the spring.sleuth.http.legacy.enabled=true flag.

Removed features

Removed classes :

• HttpTraceKeysInjector

Hystrix

The SleuthHystrixConcurrencyStrategy will always creates a new span. Brave doesn't like continuing spans too much and it makes things more complex. SleuthHystrixConcurrencyStrategy will also not add any additional tags.

Changed constructor of SleuthHystrixConcurrencyStrategy

Before

public SleuthHystrixConcurrencyStrategy(Tracing tracing, TraceKeys traceKeys)

After

public SleuthHystrixConcurrencyStrategy(Tracing tracing,
			SpanNamer spanNamer, ErrorParser errorParser)

Web Servers

No span id == new trace

There was a case in Sleuth where one passed the trace id, the debug flag was set to 1 and there was no span id. In that case we continued the span. With Brave, a new trace id will be created.

No trace id == invalid span

There was a case in Sleuth where one passed the span id, the debug flag was set to 1 and there was no trace id. In that case we continued the span. With Brave this is an exceptional situation and the debug flag will get ignored. The sampling decision will take place via the provided Sampler.

No response code tags for parent span

For the first span ever we don't set the all the response codes anymore. We're tagging status codes from 100-200 and 400+.

No support for multiple value headers

Brave doesn't support a list of values in the header. Since we delegate the header parsing to Brave we don't support those anymore either.

Removed features

TraceFilter

• removed addRequestTags
• removed addResponseTags

Sleuth Stream

This feature is completely removed. If you want to send spans via messaging use the zipkin client with kafka or rabbit dependency.

Documentation

There is no longer CR, CS, SS, SR logs. We're using Zipkin v2. Now span.kind tells you how to interpret span.timestamp, duration and remoteEndpoint. Kind in CLIENT SERVER PRODUCER CONSUMER