refactor(plugin): create one plugin instance per invocation

[ParidelPooya]

Two commits: a silent-record-loss fix, then the contract change that removes the reason the bug was possible. Supersedes and closes #734.

Commit 1 — fix(insight): key export scheduling by execution ARN

One plugin instance served every execution its environment hosted, and Lambda Managed Instances makes concurrent executions in one environment routine.

_ExportScheduler held one _pending record for the whole plugin and overwrote it regardless of which execution the record belonged to. Each record is a complete snapshot of one execution, so a newer record for the same execution supersedes the older one safely — a record for a different execution supersedes nothing.

Measured before the change, 20 trials per row:

concurrent executions	terminal records lost per trial
2	0.8 of 2 (16 of 20 trials lost one)
3	1.9 of 3
5	3.9 of 5
10	8.5 of 10

With 5 concurrent executions and a 200 ms exporter, 1 of 5 terminal records was exported and every drain() still returned without error, so the loss was silent.

Pending records are now keyed by execution ARN, with a per-execution lane holding that execution's sequence numbers and waiter count. Coalescing happens only within one execution, and drain(execution_arn) returns only once that execution's own record has reached every exporter and a flush covering it has completed. One worker thread and one export() at a time are unchanged.

Five further defects, all found while reviewing that change:

1. A BaseException from a customer exporter killed the worker between consuming a record and publishing its bookkeeping. The worker slot stayed occupied by a dying thread, _ensure_worker_locked refused to start a replacement, and the parked drain hung the invocation thread permanently. asyncio.CancelledError inherits from BaseException, so an exporter that touches asyncio can trigger this without writing raise.
2. A drain could request a second flush while one was already in flight, so a flush() ran after drain() had returned — an exporter called after the invocation went back to Lambda — and made the suite flaky in about 4% of runs.
3. The per-execution lock was not reentrant while a displaced record was released under it, so a record whose finalizer re-entered a hook for the same execution self-deadlocked the invocation thread.
4. The closed gate was a check-then-act. Customer code running under the now-reentrant lock could complete on_invocation_end on the same thread, after which the outer frame still scheduled its RUNNING record behind the terminal one. Reproduced deterministically from a single hook call.
5. A flush happened only when a record was emitted, where JS and Java flush once per sampled-in invocation end.

InsightExporter.flush also had no docstring; it now states the cadence, the exclusivity guarantee, that a flush may cover other executions' records, what happens when an exporter omits the method, and how failures are handled.

Verified: terminal-record loss after the change is 0 in every configuration up to 10 concurrent executions, at both exporter latencies, in both scheduler-level and plugin-level probes. All eight BaseException type-and-site combinations now return instead of hanging. Mutation coverage of the scheduler and gate went from 4 of 12 targeted single-line mutations caught to 11 of 17.

Commit 2 — refactor(plugin)!: create one plugin instance per invocation

The fix above is correct but the contract that required it is not. It pushed a concurrency problem onto every plugin author, and both bundled plugins had got it wrong.

The SDK now builds one plugin per invocation and drops it when the invocation ends. plugins takes factories:

exporter = Exporter()  # shared by every invocation


@durable_execution(plugins=[lambda info: MyPlugin(exporter)])
def handler(event, context): ...

A factory is any callable taking InvocationStartInfo and returning a plugin. Environment-lifetime state stays in a callable class or a closure; per-invocation state becomes ordinary instance attributes.

Why this deletes the old path instead of adding to it

While an instance path exists, a plugin cannot delete its ARN-keyed dict, because it must still work when handed to the old path. Deleting the dict is the entire point.

Removed: DurableInstrumentationPluginProvider, DURABLE_INSTRUMENTATION_PLUGIN_API_VERSION.
Added: DurableInstrumentationPluginFactory.

Entry points in the aws_durable_execution.plugins group now name a callable, so environment-based loading works unchanged. load_configured_plugins dedups explicit against environment-selected plugins by factory identity rather than by plugin type.

Plugin changes

Workflow Insight holds no ARN-keyed structure at all. Two things deliberately stay: the RLock, and the pre-hand-off re-check. Hooks dispatch sync=True on the producing thread, so races within a single invocation remain; and schedule() releases displaced records while holding the lock, which can re-enter through a customer finalizer.

The OTel plugin keeps each invocation's spans in plain attributes, so the state-reset machinery is gone.

One real defect this refactor introduced, found and fixed here

A logging handler lives as long as the execution environment; a plugin instance now lives for one invocation. The filter installed by the first invocation's plugin kept querying that discarded instance, which reports no span context once its invocation has ended. Log correlation would have stopped silently after the first invocation, and the dead plugin would have stayed reachable for the life of the environment.

install_log_filter now rebinds the installed filter to the current invocation's plugin, covered by test_install_log_filter_rebinds_to_the_current_invocations_plugin. The Java OTel plugin was checked for the same shape and does not have it: MdcSpanEnricher is static and reads Span.current().

Verified

Suite	Result
core SDK	1679 passed
Insight	86 passed
OTel	257 passed (was 239 passed, 6 failed, 1 collection error)
testing	1388 passed
conformance-tests	10 passed
conformance-tests-otel	51 passed
examples	87 passed, 2 skipped
ruff check / format	clean
mypy	clean on every file touched

Neither conformance suite imports the handlers it ships, so the suite counts alone do not prove the migration. All 23 plugin handlers and all 24 OTel handlers were additionally imported under both plugin modes (96 imports, 0 failures), and three plugin handlers were run end to end through DurableFunctionTestRunner with assertions on their emitted JSON, including both multi-plugin cases.

BREAKING CHANGE: plugins accepts factories instead of plugin instances, and the provider class is removed. Pass lambda info: MyPlugin() where you passed MyPlugin(), and point entry points at a callable instead of a provider.

Not in this PR

A BaseException out of exporter.flush() still ends the worker mid-flush. The waiting invocation is released by a replacement worker, so it no longer hangs, but the remaining exporters in that flush are skipped.

Non-terminal Workflow Insight record fields still differ across languages: JS and Java stamp endTime and durationMs on a RUNNING record and pass executionError, Python omits all four. That is a product decision, tracked separately.

---

Commit 3 — review response

A reviewer filed three blocking findings. All three are addressed in fix(plugin): give each invocation its own plugin session and log correlation. The first meant the contract change did not deliver what it promised under concurrency, so it is the most important commit in this PR.

Concurrent invocations overwrote each other's plugin session. durable_execution built one PluginExecutor at decoration time and every invocation shared it, while the executor stored that invocation's plugin instances, invocation metadata and operations provider on itself. The SDK created one plugin per invocation and then parked it in a shared mutable slot, so the refactor's whole purpose was defeated.

Measured with two invocations held inside their user function at once, before the fix:

plugin_a_events: ['start:request-a']
plugin_b_events: ['start:request-b', 'user:operation-from-b', 'user:operation-from-a', 'end:request-b:SUCCEEDED']
late_a_hook_reached_b: True

Invocation A's operation hook reached B's plugin, and A's plugin never received its own operation or end hook. A second scenario was worse: A's scope exit cleared the shared state while B was live, and B lost on_invocation_end entirely. So the defect misrouted hooks and let either invocation's teardown silently disable instrumentation for the other.

A new handler-lifetime PluginHost holds nothing but the resolved factory list and hands out a fresh PluginExecutor per invocation, matching the shape JS and Java already had. The executor is reachable only from the frames of the invocation that owns it, and a second run() raises, so reuse fails loudly rather than as silent crosstalk. The thread pool is per invocation too: sharing one would let an ending invocation shut down a pool a concurrent invocation was still submitting to. Pinned by test_durable_execution_keeps_overlapping_invocations_isolated, which uses a threading.Barrier(2) so both invocations are provably live; it failed 10 of 10 runs before the fix and passed 50 of 50 after.

OTel log enrichment used the most recently bound invocation. The filter held one mutable plugin reference, so with two invocations open the last to start won and every other invocation's records carried its trace.

Reading the active span from the OTel context, as the Java plugin's static MdcSpanEnricher does, is not sufficient here, and this is worth recording because it is the obvious fix and it does not work. Measured: at top level in the handler body, trace.get_current_span() reports no valid span, while the record is nevertheless correctly stamped from the plugin's registry fallback. The invocation span is never attached to the OTel context. Worse, the SDK runs the handler body on a pool thread created after on_invocation_start ran on the Lambda handler thread, and Python does not propagate context into new threads, so a plain ContextVar set at invocation start is invisible where records are actually emitted.

The filter is therefore stateless and resolves per record: the invocation that claimed the emitting thread through a ContextVar, else the single open invocation when exactly one is open, else nothing. Plugins claim at invocation start and again from the hooks that run on user-code threads. The open set is a WeakSet and claims are liveness-checked, so a plugin whose end hook never ran cannot keep itself alive and a pooled thread cannot correlate a later record to a finished invocation. Installation is serialized so concurrent first-time callers cannot stack filters.

Concurrent empty drains returned before a flush completed. _flush_in_flight used 0 for both "no flush in flight" and "a flush in flight covering zero exports". An invocation that emitted no record could not tell its flush was already running, so it requested a second one that then ran after both invocations had returned — customer exporter code past the invocation boundary, which this PR's own flush contract forbids. Absence is now None and coverage an integer, applied at all five sites.

Deliberately not changed, with reasons

One residual log-correlation gap. With two or more invocations open, a record emitted on the SDK's user-code pool thread before the first operation or user-function hook has run on that thread is left unstamped rather than mis-attributed. Closing it needs the core SDK to propagate contextvars.copy_context() into its handler-pool submissions in execution.py. That is one line, but it changes core SDK threading semantics for every customer, not just for this plugin, so it does not belong in a review-response commit. Leaving a record unattributed was chosen over attributing it to another customer's execution.

Registering both OTel views at once. A handler that registers InvocationOtelPluginFactory and ExecutionOtelPluginFactory together has two providers open for one invocation, so the single-open-invocation fallback goes silent and only claimed threads correlate. Every test in the package parametrizes one view or the other, and the configuration is not a documented pattern, so it is treated as secondary rather than adding invocation-identity keys to the registry.

BaseException from exporter.flush(). Still ends the worker mid-flush. The waiting invocation is released by a replacement worker so it no longer hangs, but the remaining exporters in that flush are skipped. Unchanged from the first commit's known follow-ups.

Version numbering. This is a breaking change and needs a major release. This repo has no BREAKING label or release-workflow validation enforcing a major bump, so that has to be applied deliberately at release time.

Verified after the review fixes

Suite	Before review fixes	After
core SDK	1679	1682
Insight	86	87
OTel	257	262
testing	1388	unchanged

Both new fixes are concurrency fixes, so the suites were repeated: ten consecutive core runs, ten OTel runs and ten Insight runs, with no flaky run. ruff check, ruff format --check and mypy clean on every file touched.

---

Commit subjects, and one parity gap found while fixing them

lint-commits failed on two commits, so both subjects were rewritten. The trees are unchanged — this was a message-only rewrite, verified by diffing before and after — and the branch was force-pushed; the prior tip was 7bfddca.

• refactor(plugin)!: … → refactor(plugin): …. This repo's linter parses type(scope): and requires the parenthesis to close the prefix, so it rejects the ! breaking marker outright. The breaking change is signalled by the BREAKING CHANGE: trailer in the body instead, which is the conventional-commits equivalent and the only form this linter accepts.
• The session commit's subject was 62 characters against a 50-character limit, so it is now fix(plugin): give each invocation its own plugin session. The log-correlation half of that commit is described in the body rather than the subject.

Bodies were also reflowed to 72 columns, which clears the line-length warnings on both. The first commit still emits those warnings; it is unchanged here, and they are warnings rather than errors.

One correction to the migration note

The BREAKING CHANGE: trailer now records something specific to Python that is worth calling out, because it differs from the TypeScript SDK: a plugin class is itself a valid factory here, as long as its constructor takes the info argument, because calling a class in Python constructs an instance. plugins=[MyPlugin] therefore works. In TypeScript the equivalent throws, and that SDK rejects a class at load time.

Deliberately deferred: validating explicit plugins entries

load_configured_plugins copies the explicit entries through without checking them, while the entry-point path raises PluginLoadError for a target that is not callable. So the two paths disagree, the same way they did in the TypeScript SDK before that PR's second review round.

The consequence here is milder than it was there, which is why this is deferred rather than fixed. A non-callable entry — a plugin instance, most likely — raises TypeError when the factory is called, and PluginExecutor._create_plugins logs it through logger.exception before skipping the plugin. So the failure is recorded on every invocation rather than passing silently; what is wrong is only that it is rediscovered per invocation instead of failing once at handler initialization.

Fixing it means adding a callable check over the explicit entries, which is a behavior change on the configuration path and not something this review round asked for. It belongs in its own change, alongside a decision about whether all three SDKs should reject the same set of inputs at the same point.

---

Second review round: top-level log correlation

The follow-up review confirmed the plugin-session, empty-flush and cross-attribution fixes, and found the one gap I had documented as deferred. It is now fixed, because the reviewer is right that it contradicted a documented behaviour rather than merely being incomplete.

What was wrong. A log record emitted at the top of a handler got no correlation at all when two invocations were open. on_invocation_start runs on the Lambda invocation thread, so the plugin's ContextVar claim lands there, but the SDK submits the handler body to a worker thread and Python does not propagate a ContextVar into a thread started that way. No hook runs on that worker before the handler's first statement — on_user_function_start is dispatched for durable operations only — so the claim never reached it, and the filter's single-open-invocation fallback is unavailable with two open. README.md claimed every record is stamped, so this was a contract violation.

The fix, in the core SDK. The handler body is now submitted as contextvars.copy_context().run(...), with the copy taken at submit time, after on_invocation_start has run on the same thread. This is the right layer: the SDK creates that thread to run user code, and without propagation any contextvar-based instrumentation is invisible there, not only this plugin. It matches asyncio.to_thread, which also runs its callable in a copy of the caller's context.

The background checkpointing thread is deliberately left starting from an empty context. It runs SDK checkpointing rather than user code, and it dispatches operation-end hooks, so copying the context there would make the invocation thread's ambient OTel context current on a thread that starts and ends spans. Sharing one copy between the two submissions is impossible regardless, because a Context cannot be entered twice concurrently. That choice is pinned by a test, not left to convention.

One side effect, closed rather than documented. get_current_span_context prefers a same-trace span that is current over the plugin's own Invocation span. Once the ambient context reached the handler thread, a top-level record named the ambient span instead — and under X-Ray active tracing with the ADOT layer that ambient span is the parent of Invocation, so records moved one level up the tree. Measured: spanId changed while the trace id stayed the same.

This PR is a plugin-lifetime refactor and should not change which span a record names, so the plugin now records the span that enclosed the invocation and excludes that one span from the preference. Anything that becomes current later is inside the invocation — an attempt span, a child context span, or a span the handler starts itself — and still wins, because it is more specific.

Remaining uncorrelated case, now documented rather than claimed away. A record emitted on a thread carrying no claim while more than one invocation is open: a thread customer code starts itself, and the SDK's checkpointing thread. With one invocation open both still resolve to it. README.md now says this instead of claiming every record is stamped.

Verified after this round

Rebased onto the branch after main was merged in, which brought the remaining Insight exporters and raised several baselines.

Suite	Result
core SDK	1717 passed
Insight	170 passed
OTel	270 passed
testing	1388 passed

lint-commits passes on all four commits. Bodies are wrapped to 72 columns, and the ! breaking marker is omitted because this repo's linter rejects it; the BREAKING CHANGE: trailer carries that signal instead.

---

Third review round

Seven findings, all fixed. Two were serious, three were correctness gaps, and two were regressions this branch introduced.

An exporter could exhaust the execution environment

A drain is released only by a flush that completed. So an exporter whose flush() raises a BaseException on every call left nothing advanced, killed the worker, and the waiting drain then started a replacement worker, which ran the same failing exporter and died the same way. Measured over three seconds: 4842 flush attempts and 3682 live threads, with the drain never returning. An earlier commit here fixed a permanent park by letting a replacement start; that turned the park into this storm.

The same finally also advanced completion when the first exporter aborted the fan-out. Measured with a failing exporter followed by a healthy one: the drain returned, the healthy exporter received nothing, and coverage read as delivered — permanently, because the snapshot was already out of the pending slot.

Both exporter call sites now contain BaseException. That is safe at these two sites specifically: the interpreter raises KeyboardInterrupt only in the main thread, threading discards a SystemExit raised in a worker thread, and nothing cancels the export worker because nothing outside the scheduler knows it exists. So a BaseException there was raised by the exporter, which makes it a defective exporter rather than an instruction to that thread. asyncio.CancelledError is the reachable case.

Consecutive worker deaths are now bounded at three, as a backstop for the scheduler's own code rather than for exporters. At the bound the scheduler latches export off, wakes every waiter and drops queued records with a warning. Releasing the waiter is the priority, because the waiter is an invocation thread inside on_invocation_end: parking it turns an instrumentation defect into a stalled customer execution, while dropping records loses instrumentation data only.

The plugin packages permitted a core version that cannot work

Both declared aws-durable-execution-sdk-python>=2.0.0 while requiring the factory contract, so pip accepted a resolution that fails at handler initialization. RELEASING.md makes version numbers this repository's responsibility, so this PR now sets the whole set: core 2.0.0→3.0.0, otel 1.0.0→2.0.0, insight 0.0.1→0.1.0, both bounds to >=3.0.0, the OTel layer sdk-version pin, and the two conformance packages' exact pins.

Insight is 0.1.0 rather than 0.0.2 for a mechanical reason: a consumer pinning ~=0.0.1 would silently accept 0.0.2 and break at run time, while 0.1.0 falls outside that range.

Two new metadata tests assert that the declared bound tracks the core major built here and that the layer pin matches the core version, so the next bump cannot leave one file behind.

Two regressions this branch introduced

The decorated handler advertised a phantom argument. functools.wraps was applied to the three-argument internal wrapper, so inspect.signature() reported a required plugin_executor and signature.bind(event, context) raised TypeError. A signature-aware runtime can reject such a handler. Fixed by copying the user function's metadata at the head of the wrapper chain.

Explicit plugins entries were never validated, while the entry-point path rejects a non-callable target. So plugins=[MyPlugin()] passed configuration and then failed on every invocation, logged and skipped — repeated telemetry loss instead of one configuration failure. Now validated at load time, naming the offending position.

One correction that matters and that differs from the TypeScript SDK: a plugin class stays valid here. Calling a class in Python constructs an instance, so plugins=[MyPlugin] is a working factory. Only non-callables are rejected. TypeScript rejects classes because calling one there throws; that reasoning does not transfer.

Verified

Suite	Result
core SDK	1730 passed
Insight	175 passed
OTel	272 passed
testing	1388 passed
conformance	10 passed
conformance-otel	51 passed

lint-commits passes on all five commits. The Insight suite was run ten times with a 0.37s spread on a 13.9s mean, so no latent hang remains.

---

Fourth review round

Thirteen threads from two reviewers. Six are fixed in f8b4f31 and one commit message was corrected. Two are accepted and queued. One is a design proposal I have pushed back on, and one follows from it.

The log filter could still attribute a record to the wrong execution

Two reviewers found two orderings, and both went through the same fallback: resolve a record to the only open invocation when the emitting thread has no live claim.

The first is a stale claim. Invocation A claims thread T. A ends, and unbind_invocation cannot reset T's ContextVar, because only the thread that set one can reset it. B starts and is the only open invocation. A customer thread started under A still carries A's claim, so the fallback runs and A's record is stamped with B's trace. Measured: the record carried B's trace and span exactly.

The second is a missing claim. A is open. B logs on its own wrapper thread before B reaches on_invocation_start, so B has no claim and the registry holds only A. Measured: B's record carried A's trace and span exactly.

Both are the outcome this PR said it chose against, which is that an unattributed record beats a record attributed to another customer's execution.

The fallback is deleted and a live claim is now required. One reviewer proposed narrowing the fallback to threads that never held a claim, which fixes the first ordering and leaves the second. Removing it fixes both.

Removing it is affordable because the threads running customer code all carry a claim, and that was measured rather than assumed. Over one execution driving a step, a parallel, a map and a customer-started thread, every record from user code carried a claim: the handler-body thread through the context copy, and branch threads through the hooks that run on them. Two threads now go unstamped whatever the invocation count — the SDK's checkpointing thread, submitted without the context deliberately, and a thread customer code starts itself. The README states both.

A raising span processor left the plugin registered forever

on_invocation_end ended the spans and exported the workflow span before releasing the invocation scope, with no finally. Both calls run customer tracer and processor code. So a raising processor skipped the release, and the plugin stayed in the open registry for the life of the environment with its OTel context attached. Measured with a processor whose on_end raises: the plugin remained registered in both plugins, and for ExecutionOtelPlugin a later record was stamped with the finished invocation.

That also made the registry untrustworthy, which is the input the resolution logic above depends on, so it is fixed in the same commit. The release and the flush now run in a finally. force_flush is contained there too, because an exception escaping a finally would replace the exception that ended the invocation and hide its cause.

The context-propagation change is now in the BREAKING CHANGE trailer

A reviewer pointed out that the trailer described the API change and understated the consequence — the same lesson the JS reviewer taught on #924. One effect I had not addressed: because the ambient context now reaches the handler thread, a span a customer starts at the top of the handler has the ambient span as its parent instead of being a root span. Under X-Ray active tracing with the ADOT layer that ambient span is the Lambda invocation span, so existing traces gain a level. My earlier fix changed which span a log record names, not span parenting. The trailer now names both this and the ContextVar visibility change.

Two documentation corrections

The factory docs presented a plugin class as the example factory, which encourages __init__(self, info) to do setup work against CONTRIBUTING.md:246. A class is still a valid factory, so only the emphasis moved: a lambda or a @classmethod is now recommended, class-as-factory noted as permitted.

A docstring cited two cross-SDK symbols as if they were on the default branches. Checking both repositories corrected half the finding: the JS createInvocationPluginRunner really is only in #924, but the Java PluginRunner does exist on main — it takes plugin instances, and #721 is what makes it per-invocation. The docstring now says that, and every other cross-SDK reference on the branch was verified against upstream.

Still open

Two accepted and queued, both P2, neither reproduced yet and so neither claimed as fixed: an unsynchronized generator/sampler install on a shared tracer, and a reentrant operation-change that can be overwritten by an older snapshot.

One design proposal, to replace the bare factory with a handler-lifetime plugin exposing new_invocation() returning a per-invocation scope. I have pushed back, and the version-bump thread follows from it. The full argument is on that thread; the short form is that new_invocation() taking no arguments gives up safe publication in the Java SDK, where PluginRunner publishes the instance with a volatile write before firing the first hook, so a constructor-assigned field is visible to the checkpoint and operation threads without being volatile.

Verified

Suite	Result
core SDK	1730 passed
Insight	175 passed
OTel	276 passed
testing	1388 passed
conformance	10 passed

lint-commits passes on all six commits. The OTel suite was run ten times with no flaky run.

---

The factory is now an object with a method

A reviewer proposed replacing the bare-callable factory with an object carrying a method, and argued for doing it in this PR rather than later. That argument decided it: shipping a bare callable in 3.0.0 and moving to an object afterwards would be two breaking changes on the same registration surface, and the second one is foreseeable today.

class DurableInstrumentationPluginFactory(Protocol):
    def create_plugin(self, info: InvocationStartInfo, /) -> DurableInstrumentationPlugin: ...

The reason is extensibility, not taste. A callable type alias has no member to add anything to, so adding a process-level hook later — a flush when the execution environment shuts down — would force the registration type from callable to object. An object with one method gains an optional second member additively.

Two naming decisions, both chosen so that no identifier changes meaning. The type keeps the name it already had here, so its meaning is refined rather than replaced. The method is create_plugin rather than new_invocation, because what it returns is still called a plugin, so the verb and the noun agree. This is the shape the Java SDK already had, as DurableExecutionPluginFactory.createPlugin(InvocationInfo), so Python and TypeScript converged onto Java rather than all three moving.

The protocol is deliberately not @runtime_checkable. An isinstance check against one tests only that the member name exists, so it accepts an object whose create_plugin is a string. The SDK also has to name what an invalid entry actually was, which a boolean cannot supply. And such a check requires every declared member, which would make the optional second member above non-additive for any caller who wrote one. _is_plugin_factory tests for a callable create_plugin structurally instead, so a factory need not import the protocol.

One reversal, and why it is an improvement

A plugin class used to be a valid factory, because calling a class constructs an instance, and an earlier round of this review asked us to document that. A class carries no create_plugin, so plugins=[MyPlugin] now fails at handler initialization, and the test that pinned the old behaviour is replaced.

That reversal resolves a second finding from the same round. CONTRIBUTING.md:246 asks for light constructors, and class-as-factory encouraged __init__(self, info) to do setup work. A factory object puts that work in the factory's own constructor, where it can fail before any invocation depends on it. A class declaring create_plugin as a @classmethod is still valid, because the requirement is the member and not the kind of object.

Migrated with it

Both bundled plugins' factory classes, 23 conformance plugin handlers each gaining a small factory beside its plugin, two examples, one testing-package e2e test that previously aborted collection of that whole suite, and the prose in four READMEs.

No handler's observable behaviour changed, and that was measured. The pre-migration handlers were extracted and run against the pre-migration SDK, then compared against the migrated run after dropping the execution ARN and wall-clock fields: 23 handlers, 89 records, 23 identical streams, 0 differing. Registration order is preserved in both handlers that register two plugins.

Verified

Suite	Result
core SDK	1738 passed
Insight	175 passed
OTel	276 passed
testing	1388 passed
conformance	10 passed
conformance-otel	51 passed
examples	87 passed, 2 skipped

Fifty handler and example modules were additionally imported under both OTel plugin modes with zero failures, and four handlers were driven end to end with assertions on their emitted JSON, including both that register two plugins. lint-commits passes on all eight commits.

---

Two deferred findings, now closed

Both were accepted and deferred earlier. Both turned out to be real, and the first had left Python as the only SDK without a guard the other two carry — backwards, for a finding that originated here.

A superseded record could overwrite a newer one

Record building runs customer callbacks while the per-execution lock is held. A callback can re-enter on_operation_change on the same thread, build a newer record and schedule it, after which the outer frame resumes and schedules its older snapshot. Measured, with a transform re-entering while the outer frame held an empty operation map:

export deferred:   [[]]            the newer ['s1'] snapshot coalesced away in the pending slot
export immediate:  [['s1'], []]    the stale snapshot exported after the newer one

After the fix both produce [['s1']].

Each invocation now counts its record builds, takes the next value before the build begins, and queues the record only while that value is still the newest. Dropping a superseded record loses nothing, because a record is a complete snapshot — the property that makes per-execution coalescing sound.

The counter is a plain int rather than an atomic, which differs from the Java port deliberately. Every increment and read happens while self._lock is held, since _emit is only reached from inside a with self._lock block, so the read-modify-write cannot interleave. Java needs an AtomicLong because two of its builds can genuinely run at once; here a second thread entering a hook blocks on that lock.

One honest limitation. The closing record is exempt from the check, and in Python that exemption is not reachable through a hook: on_invocation_end sets the closed gate before the closing build, and all three hooks check the gate before reaching _emit. It is kept for parity with the JS and Java ports, and so the closing record's survival does not depend on that gating holding for every future hook. It is pinned by mutation rather than by a reachable scenario.

Wrapper installation on a shared tracer was a race

TracerProvider.get_tracer caches by instrumentation scope, so two plugins built for two concurrent first invocations receive the same Tracer object. The installs were a check-then-set with no lock, so both could wrap the provider's original, and the loser would keep a wrapper the tracer no longer holds — its deterministic overrides ignored, span ids random, cross-invocation stitching broken.

The window is a few bytecodes, so 400 trials at the default switch interval produced nothing. Lowering the switch interval, which preempts inside the existing window rather than creating one:

Measurement	Before	After
rival id-generator wrappers	448 / 2000	0
rival sampler wrappers	191 / 2000	0
plugin holding a discarded wrapper	11 / 200 per factory	0

Both installs now hold a module-level lock across the read, the check, the construction and the assignment, and each returns whatever the tracer holds, so the loser uses the winner's wrapper. That follows the precedent already in this package: the log filter's install is guarded the same way.

Verified

Suite	Result
core SDK	1738 passed
Insight	178 passed
OTel	278 passed
testing	1388 passed

Both are concurrency fixes, so each suite was run ten times: Insight spread 0.26 s on a 13.8 s mean, OTel spread 0.63 s on a 2.9 s mean, no failures and no run standing out.

---

A factory's return is now validated

The load-time shape check establishes only that a factory has a callable create_plugin. What that call returns is knowable only when it runs, and _create_plugins rejected None but accepted every other object. So a factory returning object() was registered and then failed every hook, producing one logged error per hook per invocation for the life of the function while providing no telemetry.

The return is now checked against DurableInstrumentationPlugin, and an invalid one is logged once and skipped — the containment the None case already had. Before adding an isinstance gate I confirmed it refuses nothing that worked: every plugin in this repository subclasses that base class, including all 25 in the conformance handlers.

The Insight README also described a flush cadence the plugin no longer has, saying an invocation that emits no record neither starts nor flushes the worker. Every sampled-in invocation end drains and flushes now, whether or not it emitted a record, which is the cadence JS and Java have and which the tests already assert. Only a sampled-out execution neither exports nor flushes.

Core SDK is 1739 passing. lint-commits passes on all nine commits with no warnings.

---

The drain no longer runs inside a hook's lock hold

A hook runs customer code while holding the plugin's _lock: the input and output transforms, a result override in _build_operations, and __del__ on any object a displaced record carried. The lock is reentrant so that code re-entering a hook on the same thread does not self-deadlock, which means a re-entrant on_invocation_end ran its drain inside the outer frame's hold. A drain waits for the export worker, and an exporter that re-enters a hook blocks that worker on the very _lock the waiting thread holds. Neither side can proceed and the invocation hangs until Lambda times it out.

Hook frames are now counted per thread, in a threading.local shared by every instance — customer code inside one execution's build can call a hook on another execution's plugin, so the frame that must run a deferred drain is the outermost one on the thread whatever instance it belongs to. on_invocation_end requests a drain instead of performing one, and the outermost frame runs it once every lock hold on the thread is released. The unnested case is unchanged: its frame is the outermost one, so its drain runs where it always did.

A second, narrower hazard is closed with it, and it is the one Java already guarded: drain() called on the export worker thread waited for work only that thread can do. It is now refused and reported, which is what ExportScheduler.refuseWaitThatWouldBlockThePump does in Java.

Two tests pin the two paths, both failing before the change. The first asserts the nested end hook's drain runs after its frame unwinds and that the lock is free at that moment — probed from another thread, because a reentrant lock tells its owner nothing. The second drives a drain from inside export() and asserts it returns, logs the refusal, and leaves the worker serving.

The log-filter claim is weak

unbind_invocation resets the ContextVar claim only on the thread that ends the invocation, because a ContextVar cannot be reset from another thread. A pool thread that is never used again therefore kept a strong reference to a finished invocation's plugin, its spans and its context tokens for the life of the execution environment, and kept a plugin whose end hook never ran reachable despite _open_invocations being a WeakSet.

The claim is now a weakref. A collected referent resolves to nothing, which is the answer the liveness check already gives for a finished invocation, so no resolution path changed. While an invocation is open the SDK holds its plugin, which is what keeps the referent alive for every record the filter resolves.

The test keeps a claimed worker running, ends the invocation, and asserts the plugin is collected. It fails when the claim is made strong again.

A factory class is rejected at registration

plugins=[MyFactory] — the factory class rather than an instance of it — passed the shape check, because create_plugin read off the class is a plain function and therefore callable. The per-invocation call supplies only the info, Python binds it to self, and the resulting TypeError is contained like any other factory failure: instrumentation is silently absent for the lifetime of the function.

Registration now binds one positional argument to the member's signature, which runs no factory code. A bound method, a @classmethod, a @staticmethod and a __call__ on an instance all bind; an instance method read off the class does not. A callable whose signature cannot be read is accepted on the member alone — inspect.signature raises for some C-implemented callables, and a missing description of a factory is not evidence of a broken one. Both the explicit plugins=[...] path and the entry-point path use the check, and both messages now name the mistake.

plugin_discovery_test.py is 43 cases, up 6: the class rejected on each registration path, a @staticmethod accepted, a create_plugin taking no argument rejected, and an unreadable signature accepted.

A docstring code block failed CI formatting

hatch fmt --check reformats code blocks inside docstrings, which the local test suites never exercise. One example in the factory protocol's docstring failed the build job on all four Python versions. Fixed, and hatch fmt --check now passes in all seven packages.

Verified

Suite	Result
core SDK	1744 passed
Insight	180 passed
OTel	279 passed
testing	1388 passed
conformance	10 passed
conformance-otel	51 passed

hatch run types:check clean. hatch fmt --check clean in every package. lint-commits passes on all thirteen commits.

---

The end hook fires on every exit, and plugin containment widens with it

The invocation wrapper fired on_invocation_end on the success path and on except Exception, so an exit by BaseException skipped it. A handler that surfaces an asyncio.CancelledError — user code awaiting a cancelled task — leaves that way, and so does a KeyboardInterrupt or a SystemExit.

The teardown around it always ran, so the plugin instances were dropped and the pool was shut down either way. What was skipped is the one hook a plugin has to finish on: Insight never drained, so the records it held for that execution were dropped, and OTel never ended the spans it had opened. Nothing failed visibly.

Every exit now fires the hook and re-raises the exception unchanged. KeyboardInterrupt and SystemExit fire it too: the hook is what a plugin needs in order to flush, a process being torn down is when flushing matters, and the work is the same bounded work any invocation end does.

Plugin-code containment widens for the same reason. The factory boundary and the hook dispatch caught Exception, so a factory or hook raising CancelledError failed an execution it was only observing and stopped the remaining plugins from running. Both now contain every BaseException except KeyboardInterrupt, SystemExit and GeneratorExit, which are instructions to the calling thread rather than reports of a plugin defect.

CancelledError is deliberately on the contained side. It does mean "stop" for the task that was cancelled, but the task here is the SDK's: nothing cancels the invocation thread, and nothing outside PluginExecutor reaches its single-worker pool. One arriving from plugin code came from the plugin's own asyncio use, which is a plugin defect. This also brings Python level with the other two SDKs — Java contains every non-fatal Throwable and rethrows only VirtualMachineError and ThreadDeath, and JS catches everything.

ErrorObject.from_exception is annotated BaseException, which is what its own helper already accepted.

Four tests, all failing before the change: the decorator firing the end hook for each of the three exception types, the factory and hook boundaries containing cancellation, and the three control exceptions still propagating from a factory.

Verified

Core SDK 1748 passing, Insight 180, OTel 279, testing 1388, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean in every package, lint-commits clean on all fourteen commits.

---

One end notification per invocation, and more factory-class shapes rejected

Widening the wrapper to BaseException exposed a second defect in the same block, and it was reachable before that widening too. The success dispatch of on_invocation_end sat inside the try that reports a handler failure, so an end hook that raised was caught as though the handler had failed and the hook ran a second time with a RETRY outcome. Later plugins were told the wrong thing about an invocation that succeeded, and an exporter exported twice. Two things can raise there: _dispatch_plugin re-raises the three exceptions that instruct the calling thread to stop, and from_dict can reject an output the handler built.

The output is now parsed inside the try and the success hook is dispatched after it, so each invocation produces exactly one end notification.

The factory-class check also went further. A signature bind accepts create_plugin(self, info=None) and create_plugin(self, *args), because the probe binds to self and what remains is satisfied, so plugins=[MyFactory] still passed for those two shapes. For a class the kind of the member now decides: a @classmethod carries __self__, a @staticmethod is identified through its descriptor, and an attribute holding a callable object takes no implicit first argument. Anything else read off a class takes self and is rejected. inspect.getattr_static walks the MRO without running a descriptor, so this still runs no factory code.

Tests: the end hook raising a control exception, asserted to produce one SUCCEEDED notification and no RETRY; and both registration paths parameterized over the plain, defaulted and variadic instance-method forms, with a callable-object attribute asserted accepted.

Merged with main

main released SDK 2.0.1 and added chained invoke to the local test runner, which conflicted with this branch on three version pins. Resolved in favour of this branch's major: core 3.0.0, OTel 2.0.0, and the conformance packages pinned to them.

Verified after the merge

Core SDK 1754 passing, Insight 180, OTel 279, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean in every package, lint-commits clean.

---

The end hook is paired with the start hook, and a refused drain flushes

Two more from the same review, both consequences of earlier commits in this PR.

A start hook raising one of the three exceptions that instruct the calling thread to stop propagates out of the dispatch loop, so plugins later in the list never receive their start hook. The invocation-end hook that the propagating exception then triggers reached them anyway, leaving a plugin to tear down state it had never been told to build. Hooks after the start hook are now dispatched to the plugins that received the start hook, which makes the pairing an invariant rather than a rule that holds on one path. In the ordinary case the two lists are identical. A plugin counts as started before its hook is dispatched, because a hook that begins and then fails may already have allocated what its end hook releases.

The refused drain now requests a flush on its way out. An exporter that re-enters a plugin hook can queue a record from the export worker, and the worker exits its loop once nothing is pending and no flush is requested — so the record reached the exporters and the worker stopped, leaving a buffering exporter holding an execution's terminal telemetry when Lambda froze the environment. An outer drain does not always close that window: the flush barrier defers a flush until records scheduled at or before the request are exported, so a record scheduled after it can be exported once the flush has already run. The request is made without waiting, which is what keeps it safe to issue from the worker, and its barrier covers the record just queued.

Both are pinned by tests that fail without them: a start hook raising a control exception followed by a lifecycle-tracking plugin that must record nothing, and the refused drain observed to flush with no external drain asking for one.

Verified

Core SDK 1755 passing, Insight 180, OTel 279, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean, lint-commits clean.

---

Two follow-ups from reviewing the commit above

The end dispatch now finishes. An end hook raising one of the three control exceptions aborted the loop, so plugins after it lost the hook that is their only chance to finish. Every plugin the loop reaches has already started, so the first such exception is held and re-raised once every plugin has been called. No other hook defers: stopping a start-hook loop early leaves later plugins with nothing to clean up, because the pairing rule withholds their end hook too.

The flush a refused drain requests is now conditional on a pending record, which fixes a loop the previous commit introduced. An exporter whose flush() re-enters a plugin hook arrives at the refused drain from inside a flush, and an unconditional request asked for the next one — which re-entered and requested again for as long as the environment lived, after the invocation had returned. A pending record distinguishes the record a re-entering hook queued from that loop, and it cannot hide a record that needs flushing, because being pending is what sets the request.

Both are pinned by tests that fail without them, and the earlier refusal test now models its path properly: the exporter queues a record for a second execution before draining, and the flush is asserted to land after that record was exported.

Verified

Core SDK 1756 passing, Insight 181, OTel 279, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean, lint-commits clean.

---

Exception groups, and the drain registered before the build

Two more from the same reviewer, both sharpening the containment boundary this PR introduced.

A BaseExceptionGroup was neither case the boundary tested. A group carrying a KeyboardInterrupt is not an instance of one, so the tuple handler naming the three did not match it and the broad handler contained the interrupt inside it. Plugin code produces such a group without asking: an asyncio.TaskGroup whose task is interrupted raises one. Both boundaries now partition what plugin code raised — control leaves are returned and re-raised, the remainder is logged as a contained plugin failure — so a group of ordinary failures is contained whole and a mixed group propagates only its control part. The end-hook dispatch catches BaseException rather than naming the three again, because everything reaching it has already been partitioned.

Insight registers its invocation-end drain on entering the hook rather than after building the record. _apply_data_content contains Exception so that a failing redactor cannot leak the raw value, but a BaseException from a transform — a CancelledError, for instance — passes through it and leaves the hook. That path skipped the drain, so records this execution had already scheduled stayed in a buffering exporter when the environment froze. _hook_frame already runs the pending drain on the way out whether the body returned or raised, so registering on entry is one line and needs no finally.

Three tests, each failing without its fix: a mixed group whose control leaf must propagate while the rest is logged, an ordinary group that must be contained, and a terminal emit failed inside a transform whose earlier record must still reach the exporters.

Verified

Core SDK 1758 passing, Insight 182, OTel 279, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean, lint-commits clean.

---

From an independent review pass

An independent reviewer drove 8 concurrent executions through the real @durable_execution lifecycle with one shared Insight factory and got 8 complete, uncontaminated terminal records; 40 concurrent executions against the scheduler lost nothing and inverted nothing; all 12 registration shapes matched the documented accept/reject matrix. No blocking defect. Four findings acted on.

A failing end hook no longer replaces the handler's failure. The failure exit fired the end hook inside its except block, so an exception out of the hook left that block before the handler's own failure was re-raised: the caller saw the plugin's exception and the real failure survived only as __context__. Measured — a handler raising ValueError with a plugin whose end hook raises SystemExit surfaced SystemExit. The hook does raise, by design: it re-raises the control exceptions it holds through the fan-out. Instrumentation does not decide what an execution failed with, so that exception is now contained and logged there and the original is re-raised unchanged.

The sync parameter and its thread pool are gone. No production caller ever passed sync=False, and the asynchronous branch skipped the end-hook fan-out rule and swallowed a control exception in a Future nobody reads — a way to opt out of the invariants this PR establishes, on a path with no caller. Hooks are dispatched on the calling thread, which is what lets a plugin set thread-affine state the SDK's own logging reads, and a test now asserts that rather than assuming it. The pool created no thread either way, because ThreadPoolExecutor spawns lazily on first submit.

The core dependency is bounded above. Both plugin packages required >=3.0.0 with no ceiling, which is the same defect one major later: the next core that changes the plugin contract installs and then fails at handler initialization. Both are now >=3.0.0,<4, with metadata tests asserting the specifier rejects the next major and still admits the next patch.

Two rationales corrected, and the drain's cost documented. Insight's lock claimed a checkpoint-path operation-change "genuinely races" the invocation-end hook; the SDK joins the checkpoint thread and the branch pools before that hook is dispatched. The lock stays — it is what makes reentrancy from customer code inside a build safe — but the comment now says which of the two it is. The log filter's bind_invocation no longer takes a process-global lock on every operation-start and user-function-start hook when the thread already carries the claim. And the Insight README documents what the reviewer measured: the invocation-end drain waits for every record any execution had pending, and one worker serializes every export and flush, so ends are released together at the slowest — ~72 ms at one execution and ~1.8 s each at 48, with a 30 ms exporter. That is the deliberate trade against releasing an end before its record reached the exporters.

Verified

Core SDK 1758 passing, Insight 184, OTel 281, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean in every package, lint-commits clean.

---

Follow-ups from the review rounds that followed

Four more fixes, each a narrowing of something earlier in this PR, and two deliberate declines.

Displaced records are released outside every lock. _emit hands a record to the scheduler while holding this instance's lock, and the hand-off displaces the record already pending for the execution. Releasing it can run a customer __del__, and a finalizer that reaches another execution's plugin blocks on that instance's lock — two threads doing that to each other hang both invocations, which the reentrant lock does not help with because it only covers re-entry on the same instance. schedule() now returns what it displaced, including the record it rejects when export is disabled, and the hook frame releases them once the outermost hook returns.

A refused drain flushes a record already taken for export. The pending-only condition was too coarse: the worker takes the record out of the pending map before it calls the exporter, so a hook re-entered from inside export() found nothing pending and left that snapshot unflushed. The condition now asks for nothing only when a flush is already in flight with nothing newly pending, which is the flush()-re-entry case that must not loop.

Diagnostics cannot fail the containment boundary. Naming a failing factory read the factory itself, so a factory whose __getattr__ raised turned a contained failure into a failed execution. Both names — the factory's and an invalid return's type — now come from one helper that reads the type and cannot raise.

The handler's failure wins over the end hook's, and dispatch is single-threaded. Covered above; the sync parameter and its unused pool are gone.

Declined, with reasons on the threads: moving the customer content transforms out of the plugin lock, which is the same hazard class but requires restructuring the build/revision/hand-off critical section that the original defect lived in — written up as an open item rather than silently dropped; and widening a flush barrier that has already been published, which trades one extra flush in an uncommon interleaving for a mechanism that can defer a committed flush indefinitely.

Verified

Core SDK 1759 passing, Insight 186, OTel 281, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean, hatch fmt --check clean in every package, lint-commits clean.

[yaythomas]

Thank you for the thoroughness here. The Barrier-based isolation test, the measured loss tables, and the BaseException reasoning for worker threads are all correct, and they are exactly the evidence a concurrency fix needs. One blocker in the log filter, one design proposal at the factory type, two items for the breaking-change note, and some nits.

Size. Commit 1 (the Insight scheduler fix) is independently correct and independently urgent, and it does not depend on the contract change.

[github-actions]

Codex AI review

Found two unresolved Workflow Insight concurrency defects: one deadlock risk and one flush-ordering data-loss risk. Both need targeted barrier-based regression tests.

Reviewed commit 3824d1b9ec6d003131b511c7de96ac90098f6020. Workflow run

[ParidelPooya]

This round: two Python API-surface fixes, and why one check is red

This round came from a language-idiom review, not a concurrency review. Two blocking findings were fixed.

1. The registration diagnostic named the metaclass

plugin_discovery.py built its error message with type(value). When the rejected entry is a class, type(value) is the metaclass, which is builtins.type for an ordinary class.

Both of the likeliest migration mistakes are classes. plugins=[MyPlugin] is the shape the previous major accepted. plugins=[MyPluginFactory] is this major's shape with the parentheses left off. Both reported builtins.type, so the message identified neither. This is the only migration diagnostic in a breaking release with no compatibility shim, and it fires at handler initialization.

A class and a function are now named by their own qualified name. The kind is stated alongside it, because a factory class and an instance of that factory class share one qualified name, and passing the class is itself a rejected shape.

Measured before and after, same inputs:

Entry	Before	After
plugins=[MyPlugin]	builtins.type	the class __main__.MyPlugin
plugins=[MyPluginFactory]	builtins.type	the class __main__.MyPluginFactory
plugins=[lambda info: ...]	builtins.function	the function __main__.<lambda>
plugins=["oops"]	builtins.str	an instance of builtins.str

Pinned by test_a_rejected_class_is_named_by_itself_not_by_its_metaclass and test_a_rejected_class_at_the_entry_point_is_named_by_itself. Both assert the class's own qualified name is present and that builtins.type is absent.

2. The Insight factory type was private

workflow_insight() declared _WorkflowInsightFactory as its return type. That name is private and absent from __all__. The package ships py.typed, so a consumer annotating the value it holds had to import a private symbol. The OTel package, changed in this same release, exports InvocationOtelPluginFactory and ExecutionOtelPluginFactory, so the two plugin packages disagreed.

The class is now WorkflowInsightPluginFactory and is exported from the package root. The README names it.

The _ExportState mixin also set five non-underscored fields: scheduled_seq, pending_record, exported_seq, exported_at, waiters. That mixin lands on WorkflowInsightPlugin, which the package exports, and those five were the exported class's only public names. The fields belong to the export scheduler and are guarded by its lock, so a public name advertised scheduler bookkeeping as part of the plugin's API. All five are now underscore-prefixed. _ExportScheduler is declared in the same module, so it still reads them directly and the one-object design is unchanged.

Pinned by test_the_factory_type_is_public_and_re_exported and test_the_plugin_exposes_no_public_attributes. The second asserts a live plugin instance has no public attribute at all.

Verification

Core 1763 pass, insight 188 pass, otel 281, testing 1596, conformance 10, conformance-otel 51. hatch run types:check clean on all 78 / 30 / 43 source files. hatch fmt --check clean in both changed packages.

Why Post Codex review is red

The job fails with invalid AI review output: comment 1 line 594 is not on the right side of a diff hunk. The reviewer anchored a comment to plugin.py:594, which this round's commits do not touch, so the posting step refused the whole review. The failure is in the posting step, not in a test. I re-ran it and it failed identically, so it is deterministic rather than transient.

I read the rejected review from the run artifact. It contains two findings, both carrying a prior_finding_id, so both are re-files of findings already declined on this PR.

Re-file 1, transforms under the plugin lock, escalated to P1 with a deadlock claim. The claim is that two overlapping invocations can each enter a shared customer callback and re-enter the other plugin, deadlocking both threads. That cannot happen here, for three reasons:

1. _lock is created in WorkflowInsightPlugin.__init__, so each invocation has its own lock. Two invocations never contend for one plugin lock.
2. A deadlock needs two locks taken in opposite orders. The order here is always the plugin's own lock first, then whatever lock the customer callback takes. Both invocations use that same order, so there is no inversion.
3. A customer transform receives a record dict. It is handed no reference to any plugin instance, so it cannot re-enter another plugin's locked region. This round removed the last public names from the plugin class, so there is now no documented attribute through which it could reach one.

The decline stands.

Re-file 2, widening the flush barrier. Declined previously and unchanged: widening an already-published _flush_barrier trades one extra flush for the possibility of deferring a committed flush indefinitely. A steady stream of other executions would then starve a waiting drain, which is the failure the current barrier was chosen to avoid.