Skip to content

Fix projectreactor instrumentation to keep span context connected#1308

Merged
devinsba merged 12 commits into
masterfrom
devinsba/reactor-fixes-manual-rebase
Apr 29, 2020
Merged

Fix projectreactor instrumentation to keep span context connected#1308
devinsba merged 12 commits into
masterfrom
devinsba/reactor-fixes-manual-rebase

Conversation

@devinsba
Copy link
Copy Markdown
Contributor

@devinsba devinsba commented Mar 11, 2020

Fixes issue where the reactive lettuce client spans can be orphaned. This would also be an issue with spring WebClient.

supersedes #1203

fixes #1100

fixes #1178

Notes on the implication:

The largest change here is the use of reactor's built in hook mechanism. This wraps (almost) every Publisher (including Operator which implements Publisher and Subscriber) with our tracing function. The scope for any given operator is the one active at the time of creation. So if I have a pre-existing Flux and attach some operators to it while scoped to a span, all of those operators will have that span as their scope as soon as they execute.

The Webflux client uses a combination of a WebClient native filter, and a very targeted instrumentation that handles copying the current span from the reactor context, to the netty channel attributes

Given the time required to make the above changes, the server instrumentation was not changed beyond making the tests pass again. Given enough time I would also update the webflux server instrumentation to use filters directly instead of the special purpose instrumentations for each handler type.

Copy link
Copy Markdown
Contributor

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

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

It's difficult to understand the impact of this change, so I'm relying a bit on the tests.

@devinsba
Copy link
Copy Markdown
Contributor Author

@tylerbenson I updated the top level comment to include an overview of the implications

Comment thread dd-java-agent/instrumentation/reactor-core-3.1/reactor-core-3.1.gradle Outdated
@devinsba devinsba force-pushed the devinsba/reactor-fixes-manual-rebase branch from 337f353 to 5095a52 Compare March 24, 2020 20:04
@devinsba devinsba marked this pull request as draft April 17, 2020 15:41
@devinsba devinsba force-pushed the devinsba/reactor-fixes-manual-rebase branch from 2036e63 to 9da6500 Compare April 23, 2020 15:44
@devinsba
Copy link
Copy Markdown
Contributor Author

Opening back up for review, working on a couple failing tests in :dd-java-agent:instrumentation:spring-webflux-5:latestDepTest

@devinsba devinsba marked this pull request as ready for review April 23, 2020 18:56
Comment thread dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy Outdated
Comment thread dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy Outdated
@devinsba devinsba force-pushed the devinsba/reactor-fixes-manual-rebase branch from 32aa51a to 90c0e72 Compare April 24, 2020 18:56
@devinsba devinsba requested review from a team and tylerbenson April 24, 2020 18:56
Copy link
Copy Markdown
Contributor

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

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

a few questions, but it is looking really nice!

@devinsba devinsba requested a review from tylerbenson April 27, 2020 20:09
@devinsba devinsba force-pushed the devinsba/reactor-fixes-manual-rebase branch from 0153835 to d206d3b Compare April 28, 2020 15:22
@devinsba devinsba requested review from a team and tylerbenson April 28, 2020 16:05
Comment thread dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy Outdated
Comment thread dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy Outdated
@devinsba devinsba force-pushed the devinsba/reactor-fixes-manual-rebase branch from d206d3b to 915b5f6 Compare April 28, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lettuce integration dropping trace No APM traces logged on canceled requests in Reactor

4 participants