akka / akka-projection

Akka Projections is intended for building systems with the CQRS pattern, and facilitate in event-based service-to-service communication.
https://doc.akka.io/docs/akka-projection/current/
Other
100 stars 34 forks source link

ThreadLocal aware context propagation for telemetry #276

Open ignasi35 opened 4 years ago

ignasi35 commented 4 years ago

(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).

ignasi35 commented 4 years ago

Started some draft work on https://github.com/ignasi35/akka-projection/commit/f8966d29af11ba48a6f5624a0da1ab0fee2eb004

ignasi35 commented 4 years ago

This is easily implemented in the case of SingleHandler but I think the other handlers may present some challenges:

patriknw commented 4 years ago

Spontaneously, we should propagate context via ProjectionContext or the external context. Not via ThreadLocal.

ignasi35 commented 4 years ago

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

ygree commented 4 years ago

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.

octonato commented 4 years ago

@ignasi35 / @ygree, is this something we need to have on v1.0.0 or can be added later?

ignasi35 commented 4 years ago

@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.