Skip to content

feat: add Workflow Insight instrumentation plugin - #632

Draft
wangyb-A wants to merge 2 commits into
mainfrom
feat/workflow-insight-plugin
Draft

feat: add Workflow Insight instrumentation plugin#632
wangyb-A wants to merge 2 commits into
mainfrom
feat/workflow-insight-plugin

Conversation

@wangyb-A

Copy link
Copy Markdown
Contributor

Summary

Adds a Workflow Insight instrumentation plugin as a new package,
packages/aws-durable-execution-sdk-python-insight/ — a port of the JS SDK's
workflowInsight() plugin (aws-durable-execution-sdk-js-insight), treated as the
reference implementation throughout. Experimental, matching the JS plugin's status.

It listens to the SDK's instrumentation hooks and emits one curated WorkflowInsight
record (schemaVersion: "1.0") per execution. The wire record keeps the JS camelCase
field names so records read identically across SDKs and land in the same stores/queries.

Behavior (mirrors the JS plugin)

  • Exporters: LambdaLogExporter default (one JSON line to the function's log group,
    carrying the name-keyed operationsByName summary) and S3Exporter (the lossless
    per-occurrence operations array; upsert-by-execution-name; none/date/
    function-name partitioning). boto3 is an extra ([s3]) since Lambda provides it.
  • Emit model: on-complete / on-failure / on-change with export coalescing —
    a newer record supersedes a pending one; exports never propagate errors into the
    execution.
  • Sampling: deterministic per-execution ARN hash; all-or-nothing per execution.
  • Content config: input/output omission or transform (redaction), include_errors
    gating operation-level error detail only, per-operation result opt-in with optional
    transform.
  • Truncation: phase 1 drops opted-in results oldest-first, phase 2 drops whole
    operations oldest-first, input/output last; per-exporter max_record_size_bytes
    measured against the exact shape each exporter emits.
  • Operation detail: top-level (default; children with parentId suppressed) vs
    full-tree; unnamed operations are dropped (JS parity).

Depends on #616 (merged)

The plugin reads InvocationInfo.execution_input / InvocationEndInfo.execution_result
introduced by #616 — the dependency floor is set to >=1.8.0 accordingly (first release
that will carry those hooks). Capability note kept in the module docstring: the operations
map is reconstructed by accumulating per-operation hooks into per-execution state (keyed
by execution ARN to isolate warm-container reuse), since Python hooks carry no
end-of-invocation operations snapshot.

Conformance validation (live, us-west-2)

Validated against the cross-SDK insight conformance suite
(aws/aws-durable-execution-conformance-tests#73, 18 requirements): 18/18 on the s3
sink and 18/18 on the cloudwatch sink
. Two known cross-SDK divergences are documented
in that suite rather than patched over here: operation ids pass through the SDK's native
blake2b[:64] format (JS uses MD5[:16]; the suite asserts ids as opaque), and the
per-operation error.name surfaces the customer error class while the record-level error
carries the SDK wrapper name (the suite asserts non-empty).

The suite's Python example handlers land in the conformance repo as a follow-up to #73
once this package is available.

Testing

  • 17 unit tests (hatch run test:all packages/aws-durable-execution-sdk-python-insight/tests/)
    covering record shaping, operations indexing, truncation phases, sampling, emit modes,
    and exporter rendering
  • hatch fmt clean; package registered in the root known-first-party
  • Live conformance runs as above (JS-parity behavior confirmed record-for-record)

Port of the JS SDK's workflowInsight() plugin as a new package,
aws-durable-execution-sdk-python-insight: listens to the SDK's
instrumentation hooks and emits one curated WorkflowInsight record
(schemaVersion 1.0, JS-identical camelCase wire format) per execution
through configurable exporters (LambdaLogExporter default with the
operationsByName summary; S3Exporter with the per-occurrence operations
array). Mirrors the JS emit model: on-complete/on-failure/on-change
scheduling with coalescing, ARN-hash sampling, content configuration
(input/output omission and transforms, include_errors, per-operation
result opt-in), two-phase truncation, top-level vs full-tree operation
detail, and unnamed-operation dropping. Uses the invocation-hook
execution_input/execution_result fields introduced in #616.
Convert the single exporters.py module into an exporters/ package with one
module per destination (lambda_log_exporter, s3_exporter) plus a private
_common helper, mirroring the JS package's src/exporters/ layout so the set
can grow to full parity (DynamoDB, Firehose, CloudWatch Logs, ...) without a
single file accreting every backend's imports. Public import paths are
unchanged: 'from ...insight import S3Exporter' and
'from ...insight.exporters import S3Exporter' both still resolve. Adds
test_exporters.py covering both exporters (previously untested).
@wangyb-A
wangyb-A deployed to ai-pr-review August 25, 2026 22:47 — with GitHub Actions Active
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime August 25, 2026 22:52 — with GitHub Actions Error
@wangyb-A
wangyb-A deployed to ai-pr-review-runtime August 25, 2026 22:52 — with GitHub Actions Active
Comment on lines +223 to +226
def on_invocation_start(self, info: InvocationStartInfo) -> None:
if not info.execution_arn:
return
state = self._get_state(info.execution_arn)

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

High: A fresh Lambda environment starts with an empty plugin state. Prior terminal operations are not re-emitted during replay, so a resumed step -> wait -> return execution produces an incomplete terminal record. Seed state from the full initial operation map after pagination, and add a two-invocation e2e test using a new plugin instance.

Comment on lines +251 to +253
def on_operation_change(self, info: OperationChangeInfo) -> None:
for op in info.operations.values():
self._accumulate(info.execution_arn, op)

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

High: on-change only accumulates operations; _emit runs solely at invocation start/end. Consequently, operation changes are never exported. Enqueue an updated record from operation-change/end hooks with latest-record coalescing, and test multiple changes within one invocation.

Comment on lines +285 to +290
def _current_execution_arn(self) -> str | None:
with self._lock:
# The most-recently created state is the in-flight execution.
if not self._state:
return None
return next(reversed(self._state))

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

High: This returns the most recently created state, not the currently started execution. If executions A and B suspend and A later resumes, B remains last in insertion order, so A's operation-end events mutate B's record. Track an explicit active ARN on every invocation start/end, or add the ARN to OperationEndInfo.

Comment on lines +279 to +281
if is_terminal:
with self._lock:
self._state.pop(info.execution_arn, None)

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

High: Only terminal invocations remove state. Every PENDING/RETRY execution retains its input and operations indefinitely, even when sampled out, although that Lambda environment may never receive its resume. Clear state after every invocation once it can be seeded from snapshots; at minimum skip sampled-out accumulation and use bounded eviction.

build-backend = "hatchling.build"

[project]
name = "aws-durable-execution-sdk-python-insight"

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

High: The repository's PyPI workflow still enumerates only core, OTel, and testing packages. This distribution will therefore never be built or uploaded by a release, making the documented pip install unavailable. Add it to both release matrices and distribution legal-file verification.

if ops is not None:
for override in ops.overrides:
self._overrides_by_name[override.operation_name] = override
self._exporters: list[InsightExporter] = list(config.exporters)

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

Medium: The default WorkflowInsightConfig supplies an empty list, so using the plugin without explicit exporters silently emits nothing despite LambdaLogExporter being documented as the default. Distinguish omitted exporters from an explicit empty list and instantiate the log exporter when omitted.

Comment on lines +229 to +232
if info.is_first_invocation and info.execution_start_time is not None:
state.start_time = info.execution_start_time
elif info.execution_start_time is not None and state.start_time is None:
state.start_time = info.execution_start_time

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

Medium: _get_state always initializes start_time, making the state.start_time is None branch unreachable. A resumed execution in a fresh environment therefore reports the resume time, corrupting duration and date partitioning. Always adopt the service-provided execution start time when present.

Suggested change
if info.is_first_invocation and info.execution_start_time is not None:
state.start_time = info.execution_start_time
elif info.execution_start_time is not None and state.start_time is None:
state.start_time = info.execution_start_time
if info.execution_start_time is not None:
state.start_time = info.execution_start_time

Comment thread pyproject.toml
[tool.ruff.lint.isort]
known-first-party = [
"aws_durable_execution_sdk_python",
"aws_durable_execution_sdk_python_insight",

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

Medium: Only Ruff knows about the new package. Root pytest testpaths and .github/scripts/type-checks.sh omit it, so standard CI runs neither its tests nor mypy. Register its test path and type-check target, and add the required cross-component e2e coverage.

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Blocking replay/state correctness gaps remain, and the new package is not fully wired into release or CI automation. Resume and cross-invocation behavior lacks effective automated coverage.

Reviewed commit 7027ada3c6fa6fb9a329e8a6d13084778ba85038. 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