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

Support rx java #243

Merged
merged 16 commits into from
Apr 4, 2016
Merged

Conversation

shivangshah
Copy link
Contributor

Adding support for RxJava: #235
We're registering a custom https://github.com/ReactiveX/RxJava/wiki/Plugins#rxjavaschedulershook [RxJavaSchedulersHook] that wraps all Action0 instances into their Sleuth representative - the TraceAction. The hook either starts or continues a span depending on the fact whether tracing was already going on before the Action was scheduled. To disable the custom RxJavaSchedulersHook set the spring.sleuth.rxjava.schedulers.hook.enabled to false.

  1. Because RxJavaPlugins is protected does not expose reset(), a wrapper had to be introduced. More details can be found here: Make RxJavaPlugins.reset() public ReactiveX/RxJava#2297
  2. The implementation (and testing) strategy was followed per HystrixConcurrencyStrategy implementation.
  3. Adding integration test cases

marcingrzejszczak and others added 12 commits April 1, 2016 15:56
1) Because RxJavaPlugins is protected does not expose `reset()`, a wrapper had to be introduced. More details can be found here: ReactiveX/RxJava#2297
2) The implementation (and testing) strategy was followed per HystrixConcurrencyStrategy implementation.
3) Adding integration test cases
(If users need one they can add it themselves)

Fixes spring-cloudgh-239
This updates to the latest release. Beyond bug fixes, there are some
notable changes:

* storage: writes are now via AsyncSpanConsumer
* storage: added elasticsearch
* zipkin-ui: constrains start time to look for traces
* zipkin-ui: assets set cache headers, increasing responsiveness
* core: debug logging available for zipkin.internal.DependencyLinker
The retryable case was already covered and tested but if the exception
is not retryable, we need to rethrow and clean up the thread state.

Fixes spring-cloudgh-240
@shivangshah
Copy link
Contributor Author

One comment I'd like to make here is when I run the integration test case I wrote by itself:
mvn test -Dtest=org.springframework.cloud.sleuth.instrument.rxjava.SleuthRxJavaIntegrationTests it runs fine. But when I run the full mvn test that one specific test fails. I couldn't find the root cause, maybe you guys can help.
@adriancole @marcingrzejszczak @dsyer

@marcingrzejszczak marcingrzejszczak merged commit beafc3a into spring-cloud:master Apr 4, 2016
shivangshah pushed a commit to shivangshah/spring-cloud-netflix that referenced this pull request May 5, 2016
This update on RxJava version is in regards to the PR that got merged [`here`](ReactiveX/RxJava#3820) for making RxJavaPlugins' `reset` method to be public. We have to work this around in Spring Sleuth. Details on the implementation can be found [`here`] (spring-cloud/spring-cloud-sleuth#243).

1.1.5
spencergibb pushed a commit to spring-cloud/spring-cloud-netflix that referenced this pull request May 6, 2016
This update on RxJava version is in regards to the PR that got merged [`here`](ReactiveX/RxJava#3820) for making RxJavaPlugins' `reset` method to be public. We have to work this around in Spring Sleuth. Details on the implementation can be found [`here`] (spring-cloud/spring-cloud-sleuth#243).

1.1.5
spencergibb added a commit to spring-cloud/spring-cloud-netflix that referenced this pull request May 6, 2016
* pull1007:
  Updating RxJava version to 1.1.5 This update on RxJava version is in regards to the PR that got merged [`here`](ReactiveX/RxJava#3820) for making RxJavaPlugins' `reset` method to be public. We have to work this around in Spring Sleuth. Details on the implementation can be found [`here`] (spring-cloud/spring-cloud-sleuth#243).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants