akka-projection
akka-projection copied to clipboard
ThreadLocal aware context propagation for telemetry
(See some of the discussion in https://github.com/akka/akka-projection/pull/230#discussion_r444898182 but most of the discussion continued offline (private link))
Short description
Support ThreadLocal
dependant SPI-implementations.
Details
Some telemetry backends rely on ThreadLocal
usage which restricts what hooks to expose on the TelemetrySpi
and how to use them. We need a hook to be invoked immediately before handler.process
and another one to happen immediately after and invoked from the same thread where the before
was invoked and always invoked (to prevent resource leaking).
Started some draft work on https://github.com/ignasi35/akka-projection/commit/f8966d29af11ba48a6f5624a0da1ab0fee2eb004
This is easily implemented in the case of SingleHandler
but I think the other handlers may present some challenges:
-
GroupHandler
processes multiple envelopes at once, so it's unclear which envelope should be passed on (or maybe all of them) - in
FlowHandler
I'm not sure we would be able to wrap the user code between invocations tobeforeSchedule
/afterSchedule
guaranteeingafterSchedule
is always invoked and that is is invoked from the same thread.
Spontaneously, we should propagate context via ProjectionContext or the external context. Not via ThreadLocal.
Spontaneously, we should propagate context via ProjectionContext or the external context. Not via ThreadLocal.
I think there is confusion about what context
means in this issue. This issue is not about the ProjectionContext
we propagate next to the envelope across the whole stream.
Instead, this issue only focuses on a short-lived, telemetry-specific context (a Any
) which can only be propagated using ThreadLocal
because of implementation limitations on the underlying libraries. As in: "in order to use library ABC.jar for tracing the creation and the release of the tracing span must be invoked on the same Thread
".
So the proposal involves only 2 calls in the TelemetrySpi where it could make sense to try to fulfill this implementation-specific request. These 2 calls in the SPI will be the new beforeSchedule
/afterSchedule
which will wrap the handler.process
invocation.
cc @ygree
In some cases the before
and after
hooks are needed. For instance, for measuring execution time when before
returns a timestamp as a context passed into after
, or for setting a tracing context (thread local) and swapping it back. The latter is needed to avoid residual contexts to avoid propagating such contexts into the next task.
@ignasi35 / @ygree, is this something we need to have on v1.0.0 or can be added later?
@ignasi35 / @ygree, is this something we need to have on v1.0.0 or can be added later?
I think we agreed that added later
was acceptable.