You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
require an execution start timestamp when deriving the durable OpenTelemetry trace ID
remove the wall-clock fallback that could generate a different trace ID on replay
disable telemetry for an invocation when InvocationStartInfo.execution_start_time is missing
add coverage for the trace-ID helper and both OTel plugin implementations
Why
The durable trace ID embeds the execution start time in its first 32 bits. Falling back to datetime.now() when the service does not provide that timestamp makes the supposedly deterministic trace ID invocation-dependent and can split one durable execution across multiple traces.
The core plugin contract remains optional because other instrumentation plugins may not require this field. The OTel plugins enforce it at invocation start. Since plugin exceptions are intentionally swallowed by the core executor, they fail closed by leaving tracing disabled and logging an actionable warning rather than raising into a partially initialized plugin lifecycle.
This is a focused, correct fix. Requiring execution_start_time when deriving the durable OTel trace ID and removing the datetime.now() fallback eliminates the real determinism bug: the trace ID's 32-bit time prefix is now taken from the replay-stable checkpointed EXECUTION start_timestamp instead of a per-invocation wall clock, so one durable execution no longer splits across multiple traces.
Verification performed:
Both callers of _to_otel_trace_id (execution_plugin.py:287, invocation_plugin.py:409) are reached only after the new info.execution_start_time is None guard, so no path passes None; the retained runtime ValueError is consistent defensive validation for the tightened non-optional signature.
The early return in on_invocation_start leaves _tracing_enabledFalse (set by the preceding _reset_state()), so every downstream hook and on_invocation_end short-circuits cleanly and no OTel context is attached — the _assert_otel_context_balanced fixture invariant holds.
import datetime remains used in both plugins; in deterministic_id_generator.py the now-unused UTC import is correctly dropped while datetime stays referenced by the annotation — no dangling or unused imports.
The three added tests exercise the ValueError path and both plugins' fail-closed disable path and match the actual message/warning strings.
Residual test/behavior risk (not a defect): the fix is deliberately fail-closed, so when the service omits StartTimestamp or the EXECUTION operation is absent from the first page of results (state.pyget_execution_operation can return None for large/paged executions), telemetry is disabled for that invocation and a WARNING is logged on each affected invocation. This is an intentional, documented tradeoff (a dropped span is preferable to a non-deterministic trace ID), but for long-running executions whose EXECUTION operation is routinely paged out it can mean repeated per-invocation warnings and no emitted spans. No test covers that specific "start time consistently unavailable across a multi-invocation execution" scenario.
Reviewed commit e10dc12690cbaa188fbfc8d32e076742ff7b94fe. Workflow run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
InvocationStartInfo.execution_start_timeis missingWhy
The durable trace ID embeds the execution start time in its first 32 bits. Falling back to
datetime.now()when the service does not provide that timestamp makes the supposedly deterministic trace ID invocation-dependent and can split one durable execution across multiple traces.The core plugin contract remains optional because other instrumentation plugins may not require this field. The OTel plugins enforce it at invocation start. Since plugin exceptions are intentionally swallowed by the core executor, they fail closed by leaving tracing disabled and logging an actionable warning rather than raising into a partially initialized plugin lifecycle.
Testing