Skip to content

Fixes orphaned spans from the lettuce reactor client#1203

Closed
devinsba wants to merge 27 commits into
masterfrom
devinsba/netty-reactor-lettuce-orphans
Closed

Fixes orphaned spans from the lettuce reactor client#1203
devinsba wants to merge 27 commits into
masterfrom
devinsba/netty-reactor-lettuce-orphans

Conversation

@devinsba
Copy link
Copy Markdown
Contributor

@devinsba devinsba commented Feb 6, 2020

Spans were being lost when scoping in reactor, this approach uses hooks to wrap most of the operators

@devinsba devinsba force-pushed the devinsba/netty-reactor-lettuce-orphans branch from 9c6128d to 06f5081 Compare February 14, 2020 15:10
Comment on lines +16 to 21
pass {
group = "io.projectreactor.ipc"
module = "reactor-netty"
versions = "[0.7.0.RELEASE,)"
extraDependency "org.springframework:spring-webflux:5.0.0.RELEASE"
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like this will break on reactor-netty 1.0. Should I put an upper bound on this now?

"$Tags.HTTP_STATUS" 500
// Because of the way reactor has been instrumented to propagate errors we end up tagging this
// on both spans
errorTags(RuntimeException, "bad things happen")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Important change

callback?.call()
// The callback span is expected to be detached from the client trace, this however means we either have
// to have the reactor instrumentation not work in this case, breaking the lettuce flow expectations, or
// we can make this code conditional here to make the test pass
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should change the parent test class to make this conditional?

@devinsba devinsba marked this pull request as ready for review February 14, 2020 16:11
@devinsba devinsba requested a review from a team as a code owner February 14, 2020 16:11
Comment on lines +28 to +29
Hooks.onEachOperator(ReactorHooksAdvice.tracingOperator())
Hooks.onLastOperator(ReactorHooksAdvice.tracingOperator())
Copy link
Copy Markdown
Contributor Author

@devinsba devinsba Feb 14, 2020

Choose a reason for hiding this comment

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

I'm not sure why but the reactor instrumentation isn't automatically applying even though the dependency is there...

Comment thread .circleci/config.yml Outdated
@devinsba
Copy link
Copy Markdown
Contributor Author

Closing to manually rebase in #1308

@devinsba devinsba closed this Mar 11, 2020
@devinsba devinsba deleted the devinsba/netty-reactor-lettuce-orphans branch March 11, 2020 17:42
@tylerbenson tylerbenson added this to the Closed milestone Apr 10, 2020
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.

2 participants