fix(otel): Python and TS Invocation handler uses default config - #66
Conversation
… 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.
The Python OTel SDK removed use_default_tracer_provider in favor of the provider_source enum; GLOBAL is the equivalent of the old flag.
The JS OTel SDK removed useDefaultTracerProvider in favor of the providerSource enum; Global is the equivalent of the old flag.
SilanHe
changed the title
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.
GitHub Actions
Inactive
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| common = (EXAMPLES_DIR / "src" / "common.py").read_text(encoding="utf-8") | ||
| assert "ExecutionOtelPlugin" in common | ||
| assert "InvocationOtelPlugin" in common | ||
| assert "OtelPluginConfig()" in common |
There was a problem hiding this comment.
Codex AI review
This assertion passes if either plugin uses OtelPluginConfig(). Reverting the invocation branch to InvocationOtelPlugin() would therefore leave the test green despite undoing this PR's behavior. Assert each complete constructor expression so both changed branches are covered.
| assert "OtelPluginConfig()" in common | |
| assert "ExecutionOtelPlugin(OtelPluginConfig())" in common | |
| assert "InvocationOtelPlugin(OtelPluginConfig())" in common |
Codex AI reviewImplementation is scoped, but the Python regression test does not specifically cover the changed constructors. Runtime compatibility still depends on upstream SDK integration tests. Reviewed commit |
| process.env.OTEL_PLUGIN_MODE === "execution" | ||
| ? new ExecutionOtelPlugin({ useDefaultTracerProvider: true }) | ||
| : new InvocationOtelPlugin({ useDefaultTracerProvider: true }); | ||
| ? new ExecutionOtelPlugin({}) |
There was a problem hiding this comment.
Claude AI review
This drops useDefaultTracerProvider: true from ExecutionOtelPlugin, but the PR title and description scope the change to the Invocation handler only (aligning InvocationOtelPlugin to the Java default). The Java reference has no ExecutionOtelPlugin at all, so "align to Java default" gives no justification for changing the execution view.
This also contradicts README.md (lines 213-215), which states the JS examples exercise both InvocationOtelPlugin and ExecutionOtelPlugin "with the tracer provider registered by the Lambda instrumentation layer." After this change the execution-view functions will auto-configure their own OTLP tracer provider instead of using the layer-registered/global provider, silently changing how execution-view telemetry is produced and exported (notably on the ADOT/X-Ray path).
Fix: if this is unintended, keep the change scoped to Invocation and restore new ExecutionOtelPlugin({ useDefaultTracerProvider: true }). If it is intended, state the execution-view change in the PR description and update the README statement so the documented behavior matches.
| OtelPluginConfig(use_default_tracer_provider=True), | ||
| ) | ||
| return InvocationOtelPlugin() | ||
| return ExecutionOtelPlugin(OtelPluginConfig()) |
There was a problem hiding this comment.
Claude AI review
This removes use_default_tracer_provider=True from ExecutionOtelPlugin, which is the only substantive behavior change in this file: the adjacent InvocationOtelPlugin() → InvocationOtelPlugin(OtelPluginConfig()) edit is cosmetic because the no-arg constructor already applied the default config. So despite the PR title/description scoping the change to the Invocation handler, the real functional change here is to the execution view — switching it from the default/global (ADOT) tracer provider to an auto-configured OTLP provider.
This change is unmentioned in the PR and mirrors the same out-of-scope switch flagged in the JavaScript handler. Fix: if unintended, restore ExecutionOtelPlugin(OtelPluginConfig(use_default_tracer_provider=True)) and keep the change limited to InvocationOtelPlugin; if intended, document the execution-view behavior change in the PR description.
| common = (EXAMPLES_DIR / "src" / "common.py").read_text(encoding="utf-8") | ||
| assert "ExecutionOtelPlugin" in common | ||
| assert "InvocationOtelPlugin" in common | ||
| assert "OtelPluginConfig()" in common |
There was a problem hiding this comment.
Claude AI review
This assertion is too weak to guard the behavior this PR changes. "OtelPluginConfig()" in common passes as long as either plugin uses an empty config, so it neither verifies that ExecutionOtelPlugin dropped use_default_tracer_provider=True nor that InvocationOtelPlugin now passes OtelPluginConfig(). A regression that reintroduced ExecutionOtelPlugin(OtelPluginConfig(use_default_tracer_provider=True)) while leaving the Invocation call untouched would still pass. The JavaScript contract test asserts the exact form for both plugins; mirror that here with exact-string checks.
| assert "OtelPluginConfig()" in common | |
| assert "ExecutionOtelPlugin(OtelPluginConfig())" in common | |
| assert "InvocationOtelPlugin(OtelPluginConfig())" in common |
Claude AI reviewThis PR aligns the Python/JS OTel example handlers away from the default/global tracer provider. Per the PR title and description, the intended change is limited to Notably, for Python the I could not verify the SDK-internal effect of this switch from this repo alone, so I cannot confirm it is a functional break. But the change to the execution view is real, unmentioned, and undocumented, and warrants author confirmation. The Python contract test was also not tightened to lock this down. Residual test risk: the README statement about both plugins using the layer-registered provider is now stale for the JS execution view (README is not in this diff, so no inline comment). Neither test asserts the absence of Reviewed commit |
3 participants
Summary
The JS and Python SDK's
InvocationOtelPluginis being aligned to the Java default — auto-configure an OTLP provider when nothing is supplied, instead of defaulting to the global/ADOT provider (see aws/aws-durable-execution-sdk-python#612).