aws / aws/aws-durable-execution-sdk-python
[otel] Allow only one OTel plugin at a time: document and validate
- Dominant language
- Python
- Stars
- 53
- Forks
- 25
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 37
Description
## Summary
`InvocationOtelPlugin` and `ExecutionOtelPlugin` are two views of the same telemetry and are not meant to run together, but nothing stops a customer from enabling both. Both providers ship as entry points, so `DURABLE_EXECUTION_PLUGINS=otel-invocation,otel-execution` loads both, and `durable_execution(plugins=[InvocationOtelPlugin(), ExecutionOtelPlugin()])` registers both. We have decided this is unsupported. It needs to be stated in the docs and enforced by the SDK instead of silently producing broken telemetry.
## Why this is not supported
- **Duplicated spans and export volume.** Each plugin creates its own Workflow and Invocation spans for the same execution, so every operation is exported twice under two different parents.
- **Conflicting OTel context scopes.** Both plugins attach the OTel context on the same thread in `on_user_function_start`. During user code the last writer wins, so each plugin's log correlation can resolve the other plugin's span.
- **Non-LIFO unwinding leaves an ended span current.** `PluginExecutor.execute_plugins` dispatches both start and end hooks in registration order, so the scopes are not unwound in reverse. Plugin A attaches token A, plugin B attaches token B; A detaches first, restoring the pre-invocation context, then B's detach restores ctxA — plugin A's just-ended span. Both plugins' registries are empty and every attach was paired with a detach, yet the thread is left with an ended span current. `ContextVar.reset` permits out-of-order resets without error, so nothing warns. Found by the automated review on #650 (P2); see https://github.com/aws/aws-durable-execution-sdk-python/pull/650.
## Acceptance criteria
### 1. Documentation
- `packages/aws-durable-execution-sdk-python-otel/README.md` states explicitly that exactly one OTel plugin may be active, with a short rationale and guidance on choosing between them. Today the wording is "select either OTel plugin by entry-point name" (lines 28-39), which reads as a preference rather than a constraint.
- The plugin section of `packages/aws-durable-execution-sdk-python/README.md` states the same constraint for both the `DURABLE_EXECUTION_PLUGINS` path and the `plugins=[...]` argument.
- Whatever error message the validation emits names both offending plugins and tells the reader to keep one, so the docs and the runtime message agree.
- The Developer Guide (https://docs.aws.amazon.com/durable-execution/) is maintained outside this repo; a doc ticket there may be needed for the observability/logging page.
### 2. Validation
- Enabling both OTel plugins is rejected at cold start, on both registration paths:
- the environment path in `plugin_discovery.load_configured_plugins` (`DURABLE_EXECUTION_PLUGINS=otel-invocation,otel-execution`)
- the explicit path (`durable_execution(plugins=[...])`), including the mixed case where one comes from the environment and the other from the decorator
- The existing duplicate guard is not sufficient: `load_configured_plugins` only skips a provider whose **concrete type** is already registered (`plugin_discovery.py:195-207`), and these are two different types, so both load today.
- The check produces a single clear diagnostic. `PluginLoadError` (`exceptions.py:65`) is the existing failure type for the discovery path.
### Implementation options to settle in this issue
**Where the mutual exclusion is expressed.** The core SDK should not hardcode OTel class names. Two candidates:
- Add an optional exclusivity/family field to `DurableInstrumentationPluginProvider` (a frozen dataclass, so an optional field is backwards compatible) and have discovery reject two providers in the same family. This does not cover the explicit `plugins=[...]` path, which has no provider.
- Declare the family on the plugin class (for example a class-level attribute read by `PluginExecutor` or by `durable_execution`), which covers both paths uniformly.
**Failure mode.** Fail fast versus warn-and-drop is a real trade-off:
- Fail fast at cold start is unambiguous and catches the misconfiguration immediately, but a customer already running both would start failing on an SDK upgrade.
- Log an error and keep only the first registration keeps the function running with correct (single-view) telemetry, but a misconfiguration can go unnoticed.
A middle option is fail fast for the explicit `plugins=[...]` path, which a developer sees in tests, and error-and-drop for the environment path, which is a deployment-time setting.
### 3. Tests
- Unit tests for `plugin_discovery`: both entry-point names configured, one from each path, and the still-supported single-plugin cases.
- A test asserting the diagnostic names both plugins.
- If the family/exclusivity metadata approach is taken, a test that an unrelated third-party plugin alongside one OTel plugin is still allowed.
## References
- #643 — context attach/detach balance, where the ordering interaction surfaced
- #650 — PR for #643; the review comment describing the non-LIFO detach across two plugins
- `packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin_discovery.py`
- `packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py` (`PluginExecutor.execute_plugins`)
Contributor guide
Assessment
This issue has not been assessed yet.