Skip to content

feat(plugin): decouple instrumentation enums from lambda service - #634

Open
zhongkechen wants to merge 2 commits into
mainfrom
codex/plugin-owned-types
Open

feat(plugin): decouple instrumentation enums from lambda service#634
zhongkechen wants to merge 2 commits into
mainfrom
codex/plugin-owned-types

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add plugin-owned InvocationStatus and OperationType enums and convert Lambda service values at the plugin boundary
  • update the OTel plugins to depend on the plugin contract and use structural attribute checks for lifecycle payloads
  • keep the unpublished instrumentation plugin API at version 1

Testing

  • hatch run dev-core:test (1,523 passed, 5 subtests passed)
  • hatch run dev-otel:test (115 passed)
  • hatch run dev-testing:test (1,365 passed)
  • hatch run dev-core:pytest packages/aws-durable-execution-sdk-python/tests/plugin_test.py packages/aws-durable-execution-sdk-python/tests/plugin_discovery_test.py (103 passed, 5 subtests passed)
  • hatch run dev-otel:pytest packages/aws-durable-execution-sdk-python-otel/tests/test_plugin_provider.py (5 passed)
  • hatch run types:check
  • hatch fmt --check for core, OTel, and testing packages

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 13, 2026 03:41 — with GitHub Actions Inactive
@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime August 13, 2026 03:41 — with GitHub Actions Failure
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 13, 2026 06:03 — with GitHub Actions Inactive
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 13, 2026 06:03 — with GitHub Actions Inactive
Comment on lines +34 to +43
class InvocationStatus(Enum):
"""Invocation outcomes exposed to instrumentation plugins."""

SUCCEEDED = "SUCCEEDED"
FAILED = "FAILED"
PENDING = "PENDING"
RETRY = "RETRY"


class OperationType(Enum):

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.

Codex AI review

[P1] Version this plugin-contract break

API-v1 payloads now contain different enum classes, so existing plugins comparing against lambda_service.OperationType or InvocationStatus silently fail. The current OTel plugin does this, causing incorrect hooks and unfinished workflow spans after a core-only upgrade. Preserve the v1 enum identity, or bump the core and updated providers to plugin API v2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Backward compatibility isn't guaranteed for the current preview releases.

from aws_durable_execution_sdk_python.plugin import (
DurableInstrumentationPlugin,
InvocationEndInfo,
InvocationStatus,

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.

Codex AI review

[P1] Require the core release that exports these enums

The OTel package still permits core >=1.8.0, whose plugin module lacks these symbols. A valid installation pinned to 1.8.0 will fail while importing the OTel provider. Raise the dependency floor to the first compatible core release, or provide backward-compatible imports and comparisons; cover this with the PyPI-core compatibility test.

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found two high-severity cross-package compatibility issues. Staggered core/OTel upgrades are currently broken, and the published-version compatibility matrix is untested.

Reviewed commit 0a0f9a6e0016e44c2226e517d3851ce791dba4fb. Workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

Review: decouple instrumentation enums from lambda service

The refactor is complete and internally consistent. I found no actionable correctness, determinism, concurrency, or serialization defects.

What I verified

  • Every info-construction site in plugin.py now wraps the service enum through _to_operation_type/_to_invocation_status (from_operation, on_user_function_start, on_operation_action, on_operation_replay, on_child_context_end, on_operation_update, from_durable_execution_invocation_output). UserFunctionEndInfo.from_start_info copies an already-converted value. No site leaks a raw lambda_service enum onto an info object.
  • The two plugin-owned enums enumerate exactly the members of the lambda_service originals (OperationType: EXECUTION/CONTEXT/STEP/WAIT/CALLBACK/CHAINED_INVOKE; InvocationStatus: SUCCEEDED/FAILED/PENDING/RETRY), so the value-based Enum(value) conversions cannot raise for current inputs, and the source fields are all non-Optional, so there is no new None.value failure path.
  • OTel plugins now import InvocationStatus/OperationType from .plugin, so their is/in comparisons and _TERMINAL_INVOCATION_STATUSES stay same-class against info.status/info.operation_type.
  • The structural hasattr swaps are equivalent to the prior isinstance checks for the real payload types: hasattr(info, "is_replay_children")UserFunctionStartInfo|UserFunctionEndInfo, and hasattr(info, "is_first_invocation") matches InvocationStartInfo (the only invocation payload passed to _extract_attributes; InvocationEndInfo never reaches it).
  • Untouched consumers are unaffected: conformance handlers use .name, examples/core e2e use .value, and the testing package operates on lambda_service Operation/OperationUpdate rather than plugin infos. OperationStatus/OperationSubType were deliberately left on lambda_service, and their consumers still compare like-for-like.

Residual risk (not blocking)

  • The two enum definitions must stay value-identical. If lambda_service gains an OperationType/InvocationStatus member that isn't mirrored in plugin.py, _to_operation_type/_to_invocation_status would raise ValueError on the checkpoint hot path (the conversion runs even when no plugins are registered, since it's evaluated before execute_plugins' early return). This is currently guarded only by test_plugin_enums_are_independent_from_service_enums; a defensive fallback or a shared source of truth would harden it, but there is no defect today.
  • Behavioral change by design: external plugin authors comparing info.status/info.operation_type against the lambda_service enums (by is or ==) will now silently mismatch, since distinct Enum classes never compare equal even with matching values. All in-repo consumers were migrated; this only affects out-of-tree code and is the intended effect of the change. Test coverage for the conversions is adequate.

Reviewed commit 0a0f9a6e0016e44c2226e517d3851ce791dba4fb. Workflow run

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.

1 participant