search

opentracing-contrib / java-spring-messaging

OpenTracing Spring Messaging instrumentation
Apache License 2.0
3 stars 11 forks source link

Unclosed Span leads to OOM and breaks references #34

Open ledor473 opened 4 years ago

ledor473 commented 4 years ago

Using the OpenTracingChannelInterceptor class may cause the application to go in OOM because the Span gets started but are not always closed. Also, since the Scope remains active, the references (wrong parent).

In the code below, the Span is created and is made active in the ScopeManager. https://github.com/opentracing-contrib/java-spring-messaging/blob/1c5c7582d65f0ce99a52496dfad42fbefb3f5d56/opentracing-spring-messaging/src/main/java/io/opentracing/contrib/spring/integration/messaging/OpenTracingChannelInterceptor.java#L71-L72

It's supposed to be closed here: https://github.com/opentracing-contrib/java-spring-messaging/blob/1c5c7582d65f0ce99a52496dfad42fbefb3f5d56/opentracing-spring-messaging/src/main/java/io/opentracing/contrib/spring/integration/messaging/OpenTracingChannelInterceptor.java#L89-L93

That logic only works if the application is using a DirectChannel but it's unlikely to work with something like the ExecutorChannel. Furthermore, it appears that Spring Cloud Stream has a DirectChannel implementation that is not actually "direct" and needs to be excluded according to the code in Sleuth.

Our issue was with the DirectWithAttributesChannel specifically, but the fix we are testing is to only activate the Span if it's a DirectChannel, pass both the Span and Scope in the headers and close both of them in the afterSendCompletion.

pavolloffay commented 4 years ago

thanks for reporting @ledor473, would you like to submit a PR to fix it?

ledor473 commented 4 years ago

@pavolloffay yes we are still testing the change but I'll send something as soon as it's fixed