Wrap async search action logic in a new trace context#88937
Wrap async search action logic in a new trace context#88937pugnascotia merged 2 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/es-search (Team:Search) |
|
Packaging test failure is unrelated. |
| ); | ||
| } finally { | ||
| submitListener.onResponse(searchResponse); | ||
| try (var ignored = threadContext.newTraceContext()) { |
There was a problem hiding this comment.
Is this needed because we are registering a new task in the next line, which will stay alive once the submit action has returned a response? Is there a way to possibly do this automatically when needed? I can see how this is going to be forgotten in other places or possibly there are already places where the same change is needed which we are not aware of yet?
There was a problem hiding this comment.
Almost all uses of task registration happen via registerAndExecute, which handles the thread content stuff automatically. I've checked all the other places which manage tasks without registerAndExecute to make sure they're doing the right thing.
I haven't yet come up with an alternative approach unfortunately. Detecting these cases is straightforward at least, because if you don't start a new context, you get an exception!
Part of #84369. Split out from #88443. This PR wraps parts logic in
TransportSubmitAsyncSearchActionin a new tracing context. This isnecessary so that a tracing implementation can use the thread context
to propagate tracing headers, but without the code attempting to set the
same key twice in the thread context, which is illegal.