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

Stephane help #1126

Merged
merged 4 commits into from
Nov 6, 2018
Merged

Stephane help #1126

merged 4 commits into from
Nov 6, 2018

Conversation

marcingrzejszczak
Copy link
Contributor

No description provided.

Stephane Maldini added 4 commits November 6, 2018 13:45
- Use onLastOperator instead of onEachOperator
- Do not instrument scalar publishers
- Revisit ReactorSleuthMethodInvocationProcessor to reduce ops overhead
- Use only one operator for the MethodInvocationProcessor
- Warning Behavior Change: @NewSpan will defer Span creation/context
- Reduce WebFilter instrumentation to 1 operator
- Reduce WebClient instrumentation to 2 operators (1 resp, 1 body)
@smaldini
Copy link
Contributor

smaldini commented Nov 6, 2018

Moar Reactor optimization + fixing thread local populated on WebClient body emissions (see Spring Cloud Sample ITIssue)

@marcingrzejszczak marcingrzejszczak merged commit d3feae1 into master Nov 6, 2018
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so much formatting change.. mind making a line comment per major thing what is important here? I would like to learn.

private static final class SpanSubscriber
implements CoreSubscriber<Object>, Subscription, Scannable {

final CoreSubscriber<? super Object> actual;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit this doublespacing is stress testing my thumbs ability to swipe up!

@anatoliy-balakirev
Copy link

After this PR it looks like tracing headers are added to the context only when subscribe is called, while previously they were added in the subscriberContext (I'm not so familiar with all this reactive stuff, but in debug I see that it was happening earlier with the old code). So my ExchangeFilterFunction, where I relied on presence of tracing headers stopped working now (again, I'm not sure if that's a bug or I just shouldn't rely on that there). So could someone clarify if that's expected or rather some regression? If it's expected - is there any option to somehow force this code to be executed before my filter is applied?
screen shot 2019-01-28 at 11 35 27

bsideup added a commit to bsideup/spring-cloud-sleuth that referenced this pull request Feb 5, 2019
…tion)

Before spring-cloud#1126, the headers were eagerly set in
`TraceExchangeFilterFunction#filter`. After it, the side effect was
moved to lazy `MonoWebClientTrace#subscribe`.

However, we have everything to instrument the request in `filter`,
and it can be done eagerly.
marcingrzejszczak pushed a commit that referenced this pull request Feb 5, 2019
* Fix regression introduced in #1126 (brave headers propagation)

Before #1126, the headers were eagerly set in
`TraceExchangeFilterFunction#filter`. After it, the side effect was
moved to lazy `MonoWebClientTrace#subscribe`.

However, we have everything to instrument the request in `filter`,
and it can be done eagerly

fixes gh-1199
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.

5 participants