-
Notifications
You must be signed in to change notification settings - Fork 17
Add entity tracing support #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3459cb4
e1d6430
5d9c093
ac4f9ac
8ac0fc6
27f6ea2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,8 @@ | |
| import com.google.protobuf.Timestamp; | ||
| import com.microsoft.durabletask.implementation.protobuf.OrchestratorService.*; | ||
|
|
||
| import io.opentelemetry.api.trace.Span; | ||
|
|
||
| import javax.annotation.Nonnull; | ||
| import javax.annotation.Nullable; | ||
| import java.time.Instant; | ||
|
|
@@ -24,14 +26,17 @@ final class TaskEntityExecutor { | |
| private final HashMap<String, TaskEntityFactory> entityFactories; | ||
| private final DataConverter dataConverter; | ||
| private final Logger logger; | ||
| private final boolean emitTraceSpans; | ||
|
|
||
| TaskEntityExecutor( | ||
| HashMap<String, TaskEntityFactory> entityFactories, | ||
| DataConverter dataConverter, | ||
| Logger logger) { | ||
| Logger logger, | ||
| boolean emitTraceSpans) { | ||
| this.entityFactories = entityFactories; | ||
| this.dataConverter = dataConverter; | ||
| this.logger = logger; | ||
| this.emitTraceSpans = emitTraceSpans; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -80,7 +85,7 @@ EntityBatchResult execute(@Nonnull EntityBatchRequest request) { | |
| TaskEntityState entityState = new TaskEntityState(this.dataConverter, initialState); | ||
|
|
||
| // Create the concrete context that collects actions | ||
| TaskEntityContextImpl context = new TaskEntityContextImpl(entityId, this.dataConverter); | ||
| TaskEntityContextImpl context = new TaskEntityContextImpl(entityId, this.dataConverter, this.emitTraceSpans); | ||
|
|
||
| // Process each operation | ||
| List<OperationResult> results = new ArrayList<>(); | ||
|
|
@@ -127,6 +132,23 @@ EntityBatchResult execute(@Nonnull EntityBatchRequest request) { | |
|
|
||
| Instant startTime = Instant.now(); | ||
|
|
||
| // Entity processing span parented on the incoming operation trace context; | ||
| // suppressed when the host emits its own (emitTraceSpans is false). | ||
| Span processingSpan = this.emitTraceSpans | ||
| ? TracingHelper.startEntityProcessingSpan( | ||
| entityName, | ||
| operationName, | ||
| false, | ||
| instanceId, | ||
| opRequest.hasTraceContext() ? opRequest.getTraceContext() : null) | ||
| : null; | ||
|
|
||
| // Signals/orchestrations this operation produces nest under the processing span (or the | ||
| // raw incoming context when spans are suppressed), so the host can link them downstream. | ||
| context.setCurrentOperationTraceContext(processingSpan != null | ||
| ? TracingHelper.getCurrentTraceContext(processingSpan) | ||
| : (opRequest.hasTraceContext() ? opRequest.getTraceContext() : null)); | ||
|
|
||
| try { | ||
| // Build the operation | ||
| TaskEntityOperation operation = new TaskEntityOperation( | ||
|
|
@@ -157,6 +179,8 @@ EntityBatchResult execute(@Nonnull EntityBatchRequest request) { | |
| entityState.commit(); | ||
| context.commit(); | ||
|
|
||
| TracingHelper.endEntityProcessingSpan(processingSpan, null); | ||
|
|
||
| logger.log(Level.FINE, "Operation '{0}' on entity '{1}' completed successfully.", | ||
| new Object[]{operationName, instanceId}); | ||
|
|
||
|
|
@@ -188,6 +212,8 @@ EntityBatchResult execute(@Nonnull EntityBatchRequest request) { | |
| // Rollback state and actions on failure | ||
| entityState.rollback(); | ||
| context.rollback(); | ||
|
|
||
| TracingHelper.endEntityProcessingSpan(processingSpan, e.getMessage()); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -219,12 +245,20 @@ private static Timestamp toTimestamp(Instant instant) { | |
| private static class TaskEntityContextImpl extends TaskEntityContext { | ||
| private final EntityInstanceId entityId; | ||
| private final DataConverter dataConverter; | ||
| private final boolean emitTraceSpans; | ||
| private final List<PendingAction> pendingActions = new ArrayList<>(); | ||
| private int committedActionCount = 0; | ||
| @Nullable | ||
| private TraceContext currentOperationTraceContext; | ||
|
|
||
| TaskEntityContextImpl(EntityInstanceId entityId, DataConverter dataConverter) { | ||
| TaskEntityContextImpl(EntityInstanceId entityId, DataConverter dataConverter, boolean emitTraceSpans) { | ||
| this.entityId = entityId; | ||
| this.dataConverter = dataConverter; | ||
| this.emitTraceSpans = emitTraceSpans; | ||
| } | ||
|
|
||
| void setCurrentOperationTraceContext(@Nullable TraceContext traceContext) { | ||
| this.currentOperationTraceContext = traceContext; | ||
| } | ||
|
|
||
| @Nonnull | ||
|
|
@@ -244,7 +278,8 @@ public void signalEntity( | |
|
|
||
| SendSignalAction.Builder signalBuilder = SendSignalAction.newBuilder() | ||
| .setInstanceId(targetEntityId.toString()) | ||
| .setName(operationName); | ||
| .setName(operationName) | ||
| .setRequestTime(toTimestamp(Instant.now())); | ||
|
|
||
| if (input != null) { | ||
| String serializedInput = this.dataConverter.serialize(input); | ||
|
|
@@ -261,6 +296,26 @@ public void signalEntity( | |
| .build()); | ||
| } | ||
|
|
||
| if (this.currentOperationTraceContext != null) { | ||
|
bachuv marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Must address — propagate the PRODUCER span context, not its parent processing context. The action is stamped with After creating the committed PRODUCER span, set the action's |
||
| signalBuilder.setParentTraceContext(this.currentOperationTraceContext); | ||
| } | ||
|
|
||
| if (this.emitTraceSpans && this.currentOperationTraceContext != null) { | ||
| String signalScheduledTime = (options != null && options.getScheduledTime() != null) | ||
| ? options.getScheduledTime().toString() : null; | ||
| Span producerSpan = TracingHelper.startEntitySignalProducerSpan( | ||
| targetEntityId.getName(), | ||
| operationName, | ||
| targetEntityId.toString(), | ||
| this.entityId.toString(), | ||
| this.currentOperationTraceContext, | ||
| null, | ||
| signalScheduledTime); | ||
| if (producerSpan != null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Must address — do not export spans for actions that are later rolled back. This PRODUCER span is ended immediately inside Please defer emission until the action is known to be committed, and add a regression test where an entity queues an action and then throws: the result should contain zero actions and export zero corresponding PRODUCER spans. Keep the existing success assertion to prove committed actions still emit exactly once. |
||
| producerSpan.end(); | ||
| } | ||
| } | ||
|
|
||
| this.pendingActions.add(new PendingAction(PendingAction.Type.SEND_SIGNAL, signalBuilder.build(), null)); | ||
| } | ||
|
|
||
|
|
@@ -278,7 +333,8 @@ public String startNewOrchestration( | |
|
|
||
| StartNewOrchestrationAction.Builder orchBuilder = StartNewOrchestrationAction.newBuilder() | ||
| .setInstanceId(instanceId) | ||
| .setName(name); | ||
| .setName(name) | ||
| .setRequestTime(toTimestamp(Instant.now())); | ||
|
|
||
| if (input != null) { | ||
| String serializedInput = this.dataConverter.serialize(input); | ||
|
|
@@ -300,6 +356,25 @@ public String startNewOrchestration( | |
| } | ||
| } | ||
|
|
||
| if (this.currentOperationTraceContext != null) { | ||
| orchBuilder.setParentTraceContext(this.currentOperationTraceContext); | ||
| } | ||
|
|
||
| if (this.emitTraceSpans && this.currentOperationTraceContext != null) { | ||
| String orchScheduledTime = (options != null && options.getStartTime() != null) | ||
| ? options.getStartTime().toString() : null; | ||
| Span producerSpan = TracingHelper.startEntityStartOrchestrationSpan( | ||
| this.entityId.getName(), | ||
| this.entityId.toString(), | ||
| instanceId, | ||
| this.currentOperationTraceContext, | ||
| null, | ||
| orchScheduledTime); | ||
| if (producerSpan != null) { | ||
| producerSpan.end(); | ||
| } | ||
| } | ||
|
|
||
| this.pendingActions.add(new PendingAction( | ||
| PendingAction.Type.START_NEW_ORCHESTRATION, null, orchBuilder.build())); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should address — make the processing span current while user entity code runs. The span is created and its context is copied manually to Durable actions, but
entity.run(operation)is never executed underprocessingSpan.makeCurrent(). As a result, spans created by user code or automatic HTTP/database instrumentation inside the entity will not be children of this entity processing span and will appear disconnected.Please mirror the existing activity execution pattern: open a
Scopefrom the processing span aroundentity.run(...), close it on every success/failure path, and then end the span. Add a test that creates a normal nested span inside an entity operation and verifies its parent is the processing span.