Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sleuth now uses Brave #829

Merged
merged 38 commits into from Jan 19, 2018

Conversation

@marcingrzejszczak
Copy link
Contributor

marcingrzejszczak commented Jan 19, 2018

with this pull request we have rewritten the whole Sleuth internals to use Brave. That way we can leverage all the functionalities & instrumentations that Brave already has (https://github.com/openzipkin/brave/tree/master/instrumentation).

Migration guide is available here: https://github.com/spring-cloud/spring-cloud-sleuth/wiki/Spring-Cloud-Sleuth-2.0-Migration-Guide

fixes #711 - Brave instrumentation
fixes #92 - we move to Brave's Sampler
fixes #143 - Brave is capable of passing context
fixes #255 - we've moved away from Zipkin Stream server
fixes #305 - Brave has GRPC instrumentation (https://github.com/openzipkin/brave/tree/master/instrumentation/grpc)
fixes #459 - Brave (openzipkin/brave#510) & Zipkin (openzipkin/zipkin#1754) will deal with the AWS XRay instrumentation
fixes #577 - Messaging instrumentation has been rewritten

marcingrzejszczak and others added 30 commits Jan 3, 2018
What's done?

- Web Server
  - Simple handler interceptor
- Web Clients
  - RestTemplate
- Slf4j
  - with Brave in place it will also work for any other implementation (e.g. log4j)

In progress

TraceCallable and TraceRunnable in order to then do stuff with asyncs and then eventually callable on mvc to make the next samples pass.
there's still the issue546 package to copy but it requires instrumentation of async rest template etc. For now, the goal is to make the async servlet support work
can't make org.springframework.cloud.brave.instrument.web.TraceWebAsyncClientAutoConfigurationTests pass due to missing client side spans. That's because the `finish()` method in brave returns a null span and the `reporter` is not called.
added some tests, added default http client parser
@marcingrzejszczak marcingrzejszczak added this to the 2.0.0.M6 milestone Jan 19, 2018
@marcingrzejszczak marcingrzejszczak merged commit 7eb374b into master Jan 19, 2018
1 check passed
1 check passed
ci/pivotal-cla Thank you for signing the Contributor License Agreement!
Details
if (log.isDebugEnabled()) {
log.debug("Adding an error tag [" + errorMsg + "] to span " + span);
}
span.tag(Span.SPAN_ERROR_TAG_NAME, errorMsg);
span.tag("error", errorMsg);

This comment has been minimized.

Copy link
@spencergibb

spencergibb Jan 19, 2018

Member

constant?

@marcingrzejszczak marcingrzejszczak deleted the issues_#711_brave branch Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.