feat(otel): fully enum-driven OtelPluginConfig - #612
Conversation
GitHub Actions
Failure
GitHub Actions
Failure
Replace InvocationOtelPlugin's legacy positional kwargs with a single OtelPluginConfig, mirroring ExecutionOtelPlugin and the JS SDK plugins. This adds use_default_tracer_provider, enable_http_instrumentation, exporter_config, and propagators to the Invocation plugin. Also align the default provider with JS: when no provider is supplied the plugin auto-configures an OTLP provider (default_use_global=False) instead of the global/ADOT provider. Pass use_default_tracer_provider =True for the global (e.g. ADOT) provider. Migrated test call sites and README snippets to OtelPluginConfig. 105/105 otel tests pass; ruff and mypy clean. BREAKING CHANGE: InvocationOtelPlugin no longer accepts positional or keyword arguments; pass an OtelPluginConfig instead, e.g. InvocationOtelPlugin(OtelPluginConfig(tracer_provider=...)).
f11491a to
4e5fe40
Compare
The two OTel example handlers used InvocationOtelPlugin() with no config. Now that the plugin defaults to auto-configuring an OTLP provider (localhost:4318), the example tests time out when no collector is running. Pass OtelPluginConfig(use_default_tracer_provider=True) so the examples use the globally configured provider (a no-op proxy under test), matching the conformance handler and the prior no-arg behavior.
Simplify the tracer-provider selection by resolving it once into an explicit ProviderSource (EXPLICIT / GLOBAL / AUTO_OTLP) instead of recomputing booleans in two places. - Drop the default_use_global parameter from create_tracer_provider; both plugins now share the same auto-OTLP default, so the seam is dead weight. - Make OtelPluginConfig.use_default_tracer_provider a plain bool (default False) instead of a tri-state Optional[bool]. - ProviderResult carries source; owns_provider is a derived property. - register_standalone_instrumentations switches on result.source rather than (config.tracer_provider, use_default_tracer_provider). Behavior is unchanged. 105/105 otel tests pass; ruff clean; mypy clean (pre-existing optional-instrumentation import warnings only).
SilanHe
changed the title
SilanHe
changed the title
Move the ProviderSource enum into otel_plugin_config and add resolve_provider_source() so the explicit > global > auto-OTLP precedence lives in one place. create_tracer_provider becomes a straight switch on the resolved source. Remove the redundant ProviderResult.owns_provider property and the dead self._owns_provider fields in both plugins.
Replace the use_default_tracer_provider boolean with a provider_source: ProviderSource field as the single driver of provider selection. tracer_provider becomes the EXPLICIT-only field; __post_init__ validates that EXPLICIT has a tracer_provider and GLOBAL/AUTO_OTLP do not. create_tracer_provider switches on config.provider_source; the redundant resolve_provider_source helper is removed. ExecutionOtelPlugin ambient-parenting now keys off ProviderSource.GLOBAL. Tests migrate the explicit-provider+flag shortcut to monkeypatching trace.get_tracer_provider with provider_source=GLOBAL, matching the existing InvocationOtelPlugin integration test. Examples updated to provider_source=GLOBAL. BREAKING CHANGE: use_default_tracer_provider is removed; pass provider_source=ProviderSource.GLOBAL (ADOT) or ProviderSource.EXPLICIT with a tracer_provider instead.
57c1904 to
327b360
Compare
SilanHe
changed the title
Default OtelPluginConfig.provider_source to ProviderSource.GLOBAL (use the globally configured provider, e.g. the ADOT Lambda layer) instead of AUTO_OTLP.
Switch create_tracer_provider on each ProviderSource explicitly with an exhaustive guard, so no branch reads as an implicit default (the default GLOBAL lives on OtelPluginConfig.provider_source).
|
conformance tests are expected to fail until aws/aws-durable-execution-conformance-tests#66 is merged |
Docstrings and an inline comment described the old AUTO_OTLP default, stating the plugin auto-configures an OTLP provider when nothing is supplied. The default is now GLOBAL (uses the globally configured, e.g. ADOT, provider); AUTO_OTLP is the opt-in for a plugin-owned provider. Comment/docstring-only; no behavior change.
* fix(otel): pass use_default_tracer_provider=True in Python Invocation handler The Python SDK's InvocationOtelPlugin is being aligned to the JS default (auto-configure an OTLP provider when nothing is supplied) instead of defaulting to the global/ADOT provider (aws/aws-durable-execution-sdk-python#612). The conformance harness relies on the global provider configured by the OTel extension, so the Python handler must now request it explicitly -- matching the TypeScript handler's InvocationOtelPlugin({ useDefaultTracerProvider: true }). Update examples/python/src/common.py to construct InvocationOtelPlugin(OtelPluginConfig(use_default_tracer_provider=True)), and assert that constructor in test_python_examples.py. * fix(otel): python examples use ProviderSource.GLOBAL The Python OTel SDK removed use_default_tracer_provider in favor of the provider_source enum; GLOBAL is the equivalent of the old flag. * fix(otel): ts examples use ProviderSource.Global The JS OTel SDK removed useDefaultTracerProvider in favor of the providerSource enum; Global is the equivalent of the old flag. * fix(otel): examples use empty config (defaults Global) Both Python and TypeScript OTel examples now construct plugins with an empty config and rely on the provider_source/providerSource default of GLOBAL, instead of naming it explicitly. --------- Co-authored-by: silanhe <hsilan@amazon.com>
GitHub Actions
Inactive
Codex AI review
Reviewed commit |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| context_extractor=xray_context_extractor, | ||
| # Custom instrumentation scope name |
There was a problem hiding this comment.
These new doc comments are stale and contradict the API this PR ships:
- They reference
use_default_tracer_provider, which this PR removes fromOtelPluginConfig. Passing it now raisesTypeError(unknown field). The same dead reference appears again in the API-reference block below (line 264:# ...and the rest of OtelPluginConfig (use_default_tracer_provider, ...)). - "When omitted, an OTLP provider is auto-configured" is incorrect: the new default is
provider_source=GLOBAL(usestrace.get_tracer_provider()), notAUTO_OTLP. Auto-configuration now requiresprovider_source=ProviderSource.AUTO_OTLP. - The
tracer_provider=Noneexample with the comment "Provide your own TracerProvider if you already have one configured" is misleading — supplying a provider now requiresprovider_source=ProviderSource.EXPLICIT, otherwiseOtelPluginConfig.__post_init__raisesValueError.
Suggest rewording these comments to describe provider_source (GLOBAL default; AUTO_OTLP to build/own an OTLP provider; EXPLICIT + tracer_provider to supply your own) and dropping the use_default_tracer_provider mentions here and at line 264.
Claude AI reviewThis PR cleanly unifies both OTel plugins on a single enum-driven Findings1. Stale/incorrect README plugin-options docs (Medium) — Residual test risk (not blocking)
Reviewed commit |
3 participants
aws/aws-durable-execution-conformance-tests#23 (comment)
Unifies both Python OTel plugins on a single
OtelPluginConfigand makes provider selection fully enum-driven viaprovider_source(EXPLICIT/GLOBAL/AUTO_OTLP, defaultGLOBAL) with per-case validation. Removes theuse_default_tracer_providerboolean and the legacyInvocationOtelPluginkwargs.