Abstract tracer implementations via an API #1757
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1757 +/- ##
=============================================
- Coverage 55.46% 33.10% -22.36%
+ Complexity 785 724 -61
=============================================
Files 141 238 +97
Lines 4383 6259 +1876
Branches 521 693 +172
=============================================
- Hits 2431 2072 -359
- Misses 1748 4000 +2252
+ Partials 204 187 -17
Continue to review full report at Codecov.
|
| * @author Marcin Grzejszczak | ||
| * @since 3.0.0 | ||
| */ | ||
| @Configuration(proxyBeanMethods = true) |
There was a problem hiding this comment.
Did you mean proxyBeanMethods = false ?
There was a problem hiding this comment.
No I didn't cause at that time I was working with the BaggageManager abstraction and I needed one to be a bean but not autowirable anywhere. Regardless, I'll have to come back to it anyways so maybe that will change
|
TL;DR; please add to the description exactly what has been or is planned to move to this new abstraction into the PR description. This includes any remote spring projects, as the abstraction itself is the trickiest part of this. I think in general people would like the idea of abstraction in, well abstract. Even if the abstraction were perfect, and more people were hired to support this, there are other support impacts of doing this. Plus, Autoconfiguration is different than making another tracing api. It is possible to auto-configure different tracers, but difficult to get abstraction to actually work, especially with fast moving apis underneath such as open telemetry. Anyway, lets get to abstraction. For example, OpenTracing abstraction had gaps in it that made use cases difficult to really port between things. For example, the opaque span context, and lack of access to internal lifecycle of the span (ex data changing after it was recorded). This resulted in many things pinning straight to Jaeger (hunch). One reason for the brave Sleuth used brave in the first place. Higher re-use, and support leverage. Not having to know which version of spring boot has which tracer api. Being able to upgrade safely, getting model improvements for free, having some folks with deeper knowledge doing the "hard parts" and ensuring those are tested. Meanwhile end users have had the benefit of that. They have their own instrumentation which works across spring boot versions and even in non spring environments. The new abstraction will only exist and be usable in the specific version of spring cloud release train. Even if lossiness and bugs are overcome, the pinning will return for any instrumentation on this api. In short, the highest risk is the new api its existence. Auto-configuration does not require abstraction always. In doing another abstraction, anything using it will be locked to it. Maybe you should note which instrumentation will use the abstract api you have here, and what compatability guarantees you will make on that? This will help move things into a position of knowledge as no one is going to read this PR and taking things on faith requires well, faith. Adding some high level descriptions of precisely the features impacted (MDC sync, context, what have you) will give a clearer sense of the risk this PR will add. |
|
Hey @adriancole TL;DR; the description is updated. From the current user perspective the impact is close to none. For the future users they have various choices to pick. Any Brave updates can be immediately applied to Sleuth. Remark about Boot / Cloud compatibility is not applicable.
I've updated the description by literaly adding " In general there are almost no visible changes for the user (as I wrote in the PR description).
I think you misunderstood the whole change, let me try to explain this again. Sleuth's new API is used for instrumentation and span lifecycle management. Sleuth was always bound to a given boot and cloud version so I think that this is not a new thing. Users can use Brave directly. Thanks to this change they can use other tracers too.
Everything is described in the docs. At this point |
Rationale
Spring Cloud Sleuth currently is an autoconfiguration over Brave. It also consists of various instrumentation mechanisms for libraries that are not supported by Brave (e.g. Spring Cloud Circuitbreaker).
We would like to abstract Brave so that Spring Cloud Sleuth becomes an autoconfiguration for any tracer implementation that is compatible with Spring Cloud Sleuth. That way Spring Cloud Sleuth in its core module would consist of an API and various tracer implementations would implement that API which would also allow automatic instrumentation of libraries that are supported by Spring Cloud Sleuth.
OpenTelemetry Support
Thanks to doing this abstraction we are able to support new tracer implementations, not only Brave. We've decided to add support for the OpenTelemetry SDK as the second one. If in the future if we decide to add new tracers then it will be just a matter of adding a new module that bridges to the Spring Cloud Sleuth one. Thanks to abstraction of tests as well we will be easily able to plug that tracer mechanism into our current suite of tests.
Design
In
spring-cloud-sleuth-corewe have created the new API in theorg.springframework.cloud.sleuth.apipackage. That API was heavily inspired and sometimes literally taken from Brave (we wanted the potential transition to be as easy as possible). In some of the API interfaces we took inspiration from the OpenTelemetry Java API.The work is still in progress because we want to create an abstraction over the messaging parts.
Changed Modules
spring-cloud-sleuth-corenow consists of the API, instrumentation (async,circuitbreaker,messaging (via spring integration),quartz,reactor,rxjava,schedulingandweb) and its autoconfiguration. It also consists some common properties and annotation configuration setup (as it used to).New Modules
New modules were created -
spring-cloud-sleuth-braveandspring-cloud-sleuth-otelandspring-cloud-starter-sleuth-otelIn
spring-cloud-sleuth-bravewe have autoconfigurations for the bridge between Sleuth and Brave and Brave autoconfiguration as such.In
spring-cloud-sleuth-otelwe have autoconfigurations for the bridge between Sleuth and OpenTelemetry and OpenTelemetry autoconfiguration as such.Reorganization of Tests
The
tests/commonmodule consists of numerous abstract tests that then would be reused by various tracer implementations to assert whether they are complaint against Spring Cloud Sleuth instrumentation requirements. That way we write test once and we apply configuration for the tracers.Additional Changes
Multiple Propagation Options
We've added a feature to allow multiple propagation context mechanisms. The
spring.sleuth.propagation.typeproperty is a list where you can provide various options such asB3(the default), but alsoAWSorW3C. OpenTelemetry also supportsJAEGERandOT_TRACER. Of course, you can also provide your own, custom mechanism.Documentation Rewrite
We've decided to rewrite the documentation to be more consistent with Spring Boot standards (cc @Buzzardo ).
How to Use It / How Does It Affect Me?
If you're a current Spring Cloud Sleuth user you will not see any changes since you will still be using Brave's API directly. If you were using
spring-cloud-starter-sleuthdependency you automatically get the Brave tracer implementation. If you usedspring-cloud-starter-zipkinonly, you need to change it into two dependencies:spring-cloud-sleuth-zipkinandspring-cloud-starter-sleuth.If you are a new user you should do the following
If you want to migrate to the Spring Cloud Sleuth API then the migration path should be relatively simple since the API resembles Brave's one a lot.
Breaking Changes
This section might be expanded in time and will end up on a wiki page.
We did our best to make the changes least demanding for our users since backward compatibility is critical to us. However since this is a major release change we've decided to perform some backward incompatible changes.
spring-cloud-starter-zipkinmodule was removed - to maintain backward compatibility you need to change it into two dependencies:spring-cloud-sleuth-zipkinandspring-cloud-starter-sleuthcoremodule now ended up inbraveunder a different package name