fix(insight): key export scheduling by execution ARN, serialize flush

[ParidelPooya]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

n/a

Description

One plugin instance serves every execution its environment hosts, 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: 2 concurrent executions lost a terminal record in 20 of 20 trials; 5 concurrent lost 2.2 per trial; 10 concurrent lost 4.7 (and 8 with a 1 ms exporter). With 5 concurrent executions and a 200 ms exporter, 2 of 5 records were exported and drain() returned successfully anyway, so the loss was silent.

What changed:

• Pending records are keyed by execution ARN in insertion order, each execution has its own completion signal, and drain(executionArn) waits for that signal rather than for the whole queue. Coalescing happens only within one execution. One pump is unchanged, so no exporter sees two records exported at once.
• flush() is now served by that same pump, between records, so an exporter never sees flush() overlap export(), including an export belonging to a different execution. Requests queued together share one flush, and the pump exports the records an invocation is waiting on before spending a flush fan-out, so a burst of simultaneous invocation ends does not pay N times for one flush. Measured with 8 ends and a 60 ms flush: 514 ms and 8 flushes before, 60 to 138 ms and 1 to 2 flushes after.
• Orphan detection inferred "nothing outstanding" from the pending map alone, which is also true of a record already taken and being exported. An exiting pump could therefore complete another execution's drain signal mid-export, and that invocation returned before its record was delivered.
• drainAll() gave up as soon as an already-seen execution queued new work, silently weakening every assertion made after it returned. It is now bounded by passes instead.
• The drain failure path left a record queued for an unrelated execution's pump to export later, out of order and after the invocation had returned.
• flush() and drain() called from the pump thread parked forever, because the only thread able to serve the wait was the one waiting. Both now report an IllegalStateException through the failure handler and return, and a refused drain loses no work.
• A null execution ARN threw NullPointerException out of onInvocationEnd into the SDK, which the hook's own javadoc forbids.
• InsightExporter.flush() had no documented contract. It now states the cadence, the exclusivity guarantee, that a flush may cover other executions' records, and how failures are handled. The same wording is going into the JS and Python exporter interfaces.

No public type changed: ExportScheduler stays package-private and ExecutionState stays a private nested class. Record fields, emit modes, sampling, truncation, the per-record exporter fan-out and the default exporter are unchanged.

Demo/Screenshots

n/a

Checklist

• I have filled out every section of the PR template
• I have thoroughly tested this change

Testing

Unit Tests

Yes. New test classes cover: every concurrent execution delivering its terminal record exactly once; a record for one execution never displacing another's; an exiting pump not releasing a drain whose record is still inside the exporters; flush() never overlapping export(); the flush cadence, including an invocation end that emits nothing and a sampled-out end; simultaneous ends sharing one flush; a request arriving during a flush not being served by it; a throwing and an Error-throwing flush() still releasing the invocation; a flush served while records keep arriving; a re-entrant flush() being refused and reported; and a null execution ARN escaping no hook.

Verification runs: insight-plugin suite green (107 tests with the throwaway review probes excluded, 198 with them, 0 failures), the concurrency-sensitive classes re-run 20 times with 20 of 20 clean, terminal-record loss 0 in every configuration up to 10 concurrent executions, and root spotless:check clean.

Integration Tests

No. The change is internal to the insight plugin's scheduler and is covered by unit tests that drive the real plugin hooks; no cross-component or deployed behaviour changes.

Examples

No, not applicable — no public API or usage pattern changed.

[github-actions]

Codex AI review · Finding arf_v1_skpr3pouoqigoq54tslwsuqgig

With multiple exporters, callbacks run on executor workers while the pump joins those workers. If a callback re-enters flush() or drain(), this check returns false: the worker waits for the current pump while the pump waits for that worker, permanently hanging the invocation. Mark fan-out callback threads as pump-dependent (for example, using a ThreadLocal around action.accept) and refuse waits from them too; add a two-exporter regression test.

[github-actions]

Codex AI review

Found one deadlock risk in the scheduler’s multi-exporter re-entrancy handling.

Reviewed commit adb57764dd7b0d927e52f756ecb9bca4eb0f5a91. Workflow run

[ParidelPooya]

Superseded by #721, which carries this fix as its first commit plus the plugin-contract change that removes the reason the bug was possible. Closing in favour of one PR per language.