fix(tracing): use context to pass trace and retrieval state to session#1059
fix(tracing): use context to pass trace and retrieval state to session#1059
Conversation
…val state to session Trace state, carried in the context passed to NewSession was not being passed into the session. This is fixed by having session.New accept a context that contains trace and retrieval state.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1059 +/- ##
==========================================
- Coverage 61.06% 61.02% -0.05%
==========================================
Files 268 268
Lines 26135 26132 -3
==========================================
- Hits 15960 15947 -13
- Misses 8501 8506 +5
- Partials 1674 1679 +5
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
lidel
changed the title
| // Temporary session closed indepentendly of cancellation ctx. | ||
| sessCtx, cancelSession := context.WithCancel(context.Background()) | ||
|
|
||
| // Preserve retrieval.State from the original request context if present | ||
| if retrievalState := retrieval.StateFromContext(ctx); retrievalState != nil { | ||
| sessCtx = context.WithValue(sessCtx, retrieval.ContextKey, retrievalState) | ||
| } | ||
| session := bs.sm.NewSession(sessCtx, bs.provSearchDelay, bs.rebroadcastDelay) |
There was a problem hiding this comment.
@gammazero do you remember why this is separate session? do we detach if from trace on purpose?
IIUC without passing trace provider discovery operations within this temporary session won't be traced, making debugging harder? Or is this decreasing the noise?
There was a problem hiding this comment.
I think that a separate context was used for the session, so that the context passes into GetBlocks would only cancel the call to session.GetBlocks and not cancel the session itself. This allows the goroutine created here to cancel/close the session when finished reading blocks.
The purpose of doing this is so that the caller does not have to explicitly cancel the context passed into this function, which otherwise is required to prevent leaking the session created here, since otherwise there is no way to cancel/close the session.
Looking at this code, it introduces a defect in that it prevents the propagation of tracing state carried by the context passed into GetBlocks. This can be fixed by making the passed in context be the parent of the session context, or by simply returning the blocks channel and depending on the caller to cancel the passed in context.
2 participants
Trace state, carried in the context passed to NewSession was not being passed into the session. This is fixed by having session.New accept a context that contains trace and retrieval state.