[jaeger/apache-http-instrumentation] TracingRequestInterceptor gets span from http request context vs tracer scope manager - #378
Conversation
…pan from http request context vs tracer scope manager Signed-off-by: Debosmit Ray <debo@uber.com>
Signed-off-by: Debosmit Ray <debo@uber.com>
| onSpanStarted(clientSpan, httpRequest, httpContext); | ||
| } else { | ||
| log.warn("Current scope is null; possibly failed to start client tracing span."); | ||
| log.warn("Current client span is null; SpanCreationRequestInterceptor possibly failed to start client tracing span."); |
There was a problem hiding this comment.
I assume we already have a test for TracingRequestInterceptor, is it possible to add a check that log.warn is not called? With the expectation that without this change such check will fail.
There was a problem hiding this comment.
that sounds like a good idea
There was a problem hiding this comment.
@yurishkuro checking for logger state was definitely (not) a fun activity haha
ended up using a test-focused in-memory logger.
Signed-off-by: Debosmit Ray <debo@uber.com>
Codecov Report
@@ Coverage Diff @@
## master #378 +/- ##
============================================
- Coverage 84.6% 84.56% -0.05%
+ Complexity 622 621 -1
============================================
Files 95 95
Lines 2468 2468
Branches 276 276
============================================
- Hits 2088 2087 -1
Misses 284 284
- Partials 96 97 +1
Continue to review full report at Codecov.
|
weird... did you remove some tests / paths? or is this just flaky? |
|
I think its being flaky -- clicked into the codecov patch report changes seem to be in:
|
|
can we have new release? |
|
preparing release jaegertracing/legacy-client-java#12 |
4 participants
SpanCreationRequestInterceptor#processcreates a new span for the outbound request andstores it as attribute in http context
Point to be noted here is that this newly created span is not marked as the current
active span in the tracer's scope manager (which is totally fine).
TracingRequestInterceptorwas trying to access this very span from the tracer's scope manager (which is not guaranteed
to be not-null). Clearly,
TracingRequestInterceptorshould be trying to access the currentspan from the context of the current outbound request and not the tracer's thread local storage.
Signed-off-by: Debosmit Ray debo@uber.com