In process context propagation for Executors #145
Assignees
Labels
instrumentation
Instrumentation: framework support, custom plugins, ...
Comments
felixbarny
added
instrumentation
felixbarny
added a commit
to felixbarny/apm-agent-java
that referenced
this issue
Jan 11, 2019
Introducing `TraceContextHolder` in the internal API as a common superclass of both `TraceContext` and `AbstractSpan`. Given an instance of this class, you can create child spans, capture exceptions and manage activations/scopes. This abstraction reliefs clients from having to differ between the case when the current activation is a `TraceContext` vs an `AbstractSpan`. A `TraceContext` would be active when the current thread does not own the lifecycle of the parent span. Otherwise an `AbstractSpan` would be active. There is rarely a case where a client would need to get the currently active span to set the name or tags. This propagation is normally done with `@Advice.Local` variables from enter to exit advices. Should the need for that arise however, and the client can ensure that the current activation is actually an `AbstractSpan`, all they need to do is to cast `tracer.getActive()` from `TraceContextHolder` to `AbstractSpan`. The public API does not differ between `Span` and `TraceContextHolder`. `ElasticApm.currentSpan()` always returns a `Span`. In case the current activation is a `TraceContextHolder`, methods like `Span#setTag` are silent noops. Calling those methods on a `Span` whose lifecycle is not owned by the caller is illegal anyway. However, calling `ElasticApm.currentSpan().createSpan()` is always allowed. Taking away `ElasticApm.currentSpan()` is not an option, as a common use case is to rename the spans created by the auto instrumentation or to add custom tags. Also, `ElasticApm.currentSpan().createSpan()` makes for a much nicer and more object-oriented API than starting a span on a tracer, passing in the parent as a start option. In preparation of elastic#145
felixbarny
added a commit
to felixbarny/apm-agent-java
that referenced
this issue
Jan 11, 2019
Closed
felixbarny
added a commit
that referenced
this issue
Jan 14, 2019
Introducing `TraceContextHolder` in the internal API as a common superclass of both `TraceContext` and `AbstractSpan`. Given an instance of this class, you can create child spans, capture exceptions and manage activations/scopes. This abstraction reliefs clients from having to differ between the case when the current activation is a `TraceContext` vs an `AbstractSpan`. A `TraceContext` would be active when the current thread does not own the lifecycle of the parent span. Otherwise an `AbstractSpan` would be active. There is rarely a case where a client would need to get the currently active span to set the name or tags. This propagation is normally done with `@Advice.Local` variables from enter to exit advices. Should the need for that arise however, and the client can ensure that the current activation is actually an `AbstractSpan`, all they need to do is to cast `tracer.getActive()` from `TraceContextHolder` to `AbstractSpan`. The public API does not differ between `Span` and `TraceContextHolder`. `ElasticApm.currentSpan()` always returns a `Span`. In case the current activation is a `TraceContextHolder`, methods like `Span#setTag` are silent noops. Calling those methods on a `Span` whose lifecycle is not owned by the caller is illegal anyway. However, calling `ElasticApm.currentSpan().createSpan()` is always allowed. Taking away `ElasticApm.currentSpan()` is not an option, as a common use case is to rename the spans created by the auto instrumentation or to add custom tags. Also, `ElasticApm.currentSpan().createSpan()` makes for a much nicer and more object-oriented API than starting a span on a tracer, passing in the parent as a start option. In preparation of #145
felixbarny
added a commit
to felixbarny/apm-agent-java
that referenced
this issue
Jan 18, 2019
felixbarny
added a commit
to felixbarny/apm-agent-java
that referenced
this issue
Jan 18, 2019
felixbarny
added a commit
that referenced
this issue
Jan 29, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To propagate the trace context to different threads, we should by default instrument
ExecutorServices to propagate the currently active span. The user should not have to manually wrap theExecutorin order to enable the context propagation for it.Prior art:
One design decision is whether we want to wrap
Runnables and store the context as an instance variable or if we want to instrument implementations ofRunnableand add an instance variable via BCI. Wrapping has the disadvantage that we are creating garbage and type casts to a specific sub-type ofRunnablewould fail. The disadvantage of adding the instance variable via BCI is that this approach would not work with runtime attachment of the agent. Yet another option would be to store the trace context in a map which associates Runnables with the corresponding trace context.Another question is whether we want to generically instrument all runnables or only those submitted to an
Executor. We could store the trace context when the runnable is created and restore it in the run method. The assumption here is that theRunnableis instantiated in the Thread which holds the current trace context. But this assumption is not always true, for example when theRunnableis stored in an instance variable.Relevant interfaces are:
java.util.concurrent.Executor,java.util.concurrent.ExecutorService,java.util.concurrent.ScheduledExecutorService,java.lang.Runnableandjava.util.concurrent.CallableThis should fix https://discuss.elastic.co/t/java-apm-agent-sends-no-timeline-spans/141150, where Hystrix is used for fault tolerance of JDBC queries.
The text was updated successfully, but these errors were encountered: