Ensure Sink.contextView is propagated #1450
Conversation
When creating a Mono via Mono.create any nested Monos should have the higher level sinks context view propogated down. JAVA-5345
|
I was unable to create a test to replicate the tracing issue reported in: spring-projects/spring-data-mongodb#4650. However, I manually tested against the repo in ticket and confirmed it works. |
|
Shouldn't we also do a similar change here? We have a collectionInfoRetriever.filter(databaseName, cryptContext.getMongoOperation())
.contextWrite(sink.contextView())
.doOnSuccess...
...
.subscribe();If "yes", then there is plenty more places that need the change. |
...ctive-streams/src/main/com/mongodb/reactivestreams/client/internal/BatchCursorPublisher.java
Outdated
Show resolved
Hide resolved
...s/src/main/com/mongodb/reactivestreams/client/internal/gridfs/GridFSUploadPublisherImpl.java
Outdated
Show resolved
Hide resolved
...s/src/main/com/mongodb/reactivestreams/client/internal/gridfs/GridFSUploadPublisherImpl.java
Outdated
Show resolved
Hide resolved
|
@stIncMale I agree its not helped by not being able to replicate the original test without the full stack. Let me investigate some more. |
...r-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/BatchCursorFlux.java
Show resolved
Hide resolved
...r-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/BatchCursorFlux.java
Show resolved
Hide resolved
...ctive-streams/src/main/com/mongodb/reactivestreams/client/internal/BatchCursorPublisher.java
Show resolved
Hide resolved
| @@ -120,7 +120,7 @@ public void subscribe(final Subscriber<? super Void> s) { | |||
| return originalError; | |||
| }) | |||
| .then(Mono.error(originalError))) | |||
| .doOnCancel(() -> createCancellationMono(terminated, timeout).subscribe()) | |||
| .doOnCancel(() -> createCancellationMono(terminated, timeout).contextWrite(ctx).subscribe()) | |||
There was a problem hiding this comment.
Cancellation has a side effect - so we need to propagate context there.
There was a problem hiding this comment.
Isn't us calling subscribe() here not with a subscriber that was given to us (the s argument), the only thing that requires us to propagate the context from the subscriber s? I suspect that the fact that createCancellationMono has a side-effect (it mutates terminated) is irrelevant. If the side-effect is relevant to context propagation, then could you please explain why?
There was a problem hiding this comment.
On cancellation of the main Publisher we are calling subscribe() on a async clean up Publisher and we ignore any errors, results / completion.
If the side-effect is relevant to context propagation, then could you please explain why?
The user may have tracing and want to trace from the web app to the driver. Without adding the context here we break that chain and they cannot tie the cleanup operations to a web request.
...s/src/main/com/mongodb/reactivestreams/client/internal/gridfs/GridFSUploadPublisherImpl.java
Show resolved
Hide resolved
...s/src/main/com/mongodb/reactivestreams/client/internal/gridfs/GridFSUploadPublisherImpl.java
Outdated
Show resolved
Hide resolved
| @@ -120,7 +120,7 @@ public void subscribe(final Subscriber<? super Void> s) { | |||
| return originalError; | |||
| }) | |||
| .then(Mono.error(originalError))) | |||
| .doOnCancel(() -> createCancellationMono(terminated, timeout).subscribe()) | |||
| .doOnCancel(() -> createCancellationMono(terminated, timeout).contextWrite(ctx).subscribe()) | |||
There was a problem hiding this comment.
Isn't us calling subscribe() here not with a subscriber that was given to us (the s argument), the only thing that requires us to propagate the context from the subscriber s? I suspect that the fact that createCancellationMono has a side-effect (it mutates terminated) is irrelevant. If the side-effect is relevant to context propagation, then could you please explain why?
|
@stIncMale answered your questions - not sure why the reply to |
When creating a Mono via Mono.create any nested
Monos should have the higher level sinks context view propogated down.
JAVA-5345