Skip to content

fix(otel): clarify registration and mode errors - #912

Open
zhongkechen wants to merge 5 commits into
aws:mainfrom
zhongkechen:fix/otel-factory-review-follow-up
Open

zhongkechen wants to merge 5 commits into
aws:mainfrom
zhongkechen:fix/otel-factory-review-follow-up

Conversation

@zhongkechen

Copy link
Copy Markdown
Contributor

Issue #, if available

Follow-up to #905.

Description of changes

Combining plugins=[create_otel_plugin_factory()] with DURABLE_EXECUTION_PLUGINS=otel raises PluginLoadError: the selector constructs a new factory, while the entry point loads the default singleton. The README now explains choosing one registration source, or using the exported singleton when both sources must name the same default view. Six integration cases cover rejection and singleton deduplication for all three aliases.

Invalid modes now include the rejected value, for example mode must be 'execution' or 'invocation', got 'EXECUTION'. The selector tests move into test_plugin_factory.py, retaining installed-metadata coverage and removing its skip. The resume test requires exactly one work span, parented to the first invocation in invocation mode. Repeated README text is shortened, and the function export is grouped with the other functions.

Design

  • Design note: not needed; no new feature, option, signature, or registration behavior.
  • Simpler options: consolidate the existing tests and documentation; no new runtime branches or registration logic.
  • Root cause: the invalid-mode error omitted the supplied value. The registration documentation and tests did not explicitly cover a fresh factory mixed with a matching installed entry point.
  • Net change in source lines: 0. Public API members added or removed: 0.

Validation and assumptions

  • The first commit adds the stronger invalid-mode assertions: 6 failed before the message fix; the same 6 passed after it.
  • hatch run dev-otel:cov: 965 passed, 95% package coverage.
  • hatch run dev-otel:typecheck: passed for 46 source/test files.
  • hatch fmt --check: passed at the root and in the OTel package.
  • Core and OTel wheel/sdist builds: passed.
  • hatch run test-wheel-otel:test: 965 OTel tests and 12 lifecycle compatibility tests passed against built wheels; installed source bytes and artifact hashes verified.
  • Registration checks use actual installed entry points, the core SDK, and OpenTelemetry SDK with in-memory exporters. Live Lambda/ADOT/X-Ray export was not exercised.

Decisions not covered by the issue or the design note

  • Retain installed-entry-point assertions: they verify installed metadata, while the existing TOML test verifies source declarations.
  • Preserve the function's snake-case name and move its export beside other functions.
  • Use the invocation's durable.invocation.first attribute rather than exporter ordering to identify the parent.

Edited test expectations

  • test_invalid_modes_are_rejected: require the supplied value in the error message.
  • test_installed_entry_points_preserve_alias_identity: require installed metadata instead of skipping when it is absent, consistent with the integration suite.
  • test_unified_factory_preserves_view_across_resume: require one work span and its precise parent, catching duplicate spans and incorrect replay parenting.
  • The other consolidated selector tests retain their assertions.

Checklist

  • No new runtime branches; the changed diagnostic fails its tests without the fix.
  • The first commit adds the failing regression assertions.
  • Every edited test expectation is listed above with its reason.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime November 6, 2026 21:02 — with GitHub Actions Active
`mode` read from configuration at runtime is accepted and type-checks as the
union of both return types.
Register only one returned factory; see the
[registration guidance](#quick-start-using-x-raycloudwatch-tracing) when using

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this "registration guidance" link targets #quick-start-using-x-raycloudwatch-tracing, which is the Quick Start heading at the top of the section (line 41). The guidance it refers to (choosing between plugins=[...] and DURABLE_EXECUTION_PLUGINS, and when to use EXECUTION_OTEL_PLUGIN_FACTORY) is actually the "Set exactly one OTel entry name" block lower down (lines 91-118). That block has no heading, so there's no anchor to point at. Consider giving it a ### heading (e.g. ### Choosing a registration source) and linking to that anchor instead, so the reader lands on the relevant text.

🤖 AI-assisted comment

This branch was successfully deployed

1 active (outdated) deployment
ai-pr-review-runtime — f9239b6f Deployed Nov 6, 2026 by zhongkechen via notify / Generate notification summary #386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants