-
Notifications
You must be signed in to change notification settings - Fork 875
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
Simplify webflux mono handling #3816
Simplify webflux mono handling #3816
Conversation
This reverts commit 484cafa.
return Operators.lift( | ||
(scannable, subscriber) -> new SpanFinishingSubscriber<>(subscriber, context)); | ||
public static <T> Mono<T> setPublisherSpan(Mono<T> mono, Context context) { | ||
return mono.doOnError(t -> finishSpanIfPresent(context, t)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on doOnError
only setting the exception and doOnTerminate
finishing the span?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that doesn't really fit the Instrumenter model though, of calling end() and passing the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Instrumenter coming in #3798)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that makes sense too
This reverts commit dacc0c1.
Splitting the more problematic (and independent) part of #3798 out into a separate PR for better scrutiny/history.
Based on @HaloFour's #3798 (comment), I realized that the change in #3789 that seems to fix the flakiness that we previously fixed via #3150, is not the change in instrumentation placement, but the registeration of a cancellation handler using Mono's
doOnCancel
.