aws / aws/aws-durable-execution-sdk-python

[otel] Allow only one OTel plugin at a time: document and validate

Ouverte
#652 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
bug documentation pkg:otel pkg:sdk
Langage dominant
Python
Étoiles
53
Forks
25
Merge moyen
1 j 19 h
PR mergées (30 j)
40

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`)

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Commencez par plugin_discovery.py, plugin.py et la définition de PluginLoadError, puis examinez les deux sections de README consacrées aux plugins ainsi que les tests existants de découverte des plugins. Suivez les parcours d’enregistrement via l’environnement et explicite, tranchez les choix concernant l’exclusivité et les modes d’échec, et vérifiez que les cas avec un seul plugin, l’enregistrement mixte, les diagnostics mentionnant les deux plugins et les plugins sans rapport sont couverts.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
backend-api-design, observability-sre
Type d'issue
Fonctionnalité
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
Active
Clarté
Plutôt claire
Accessibilité débutants
42/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.