Skip to content

feat(otel): add structured trace-context types and execution-trace resolver - #685

Open
ayushiahjolia wants to merge 1 commit into
mainfrom
otel-shared-execution-trace
Open

feat(otel): add structured trace-context types and execution-trace resolver#685
ayushiahjolia wants to merge 1 commit into
mainfrom
otel-shared-execution-trace

Conversation

@ayushiahjolia

@ayushiahjolia ayushiahjolia commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available: #674

Description of changes:
First PR in the stack that reparents durable OTel spans onto one shared execution trace. Adds the foundational data types only - nothing consumes them yet, so behavior is unchanged.

Next in stack: durable sampling, then wiring both plugins onto the shared trace.

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from 9ff9f55 to b645aea Compare August 27, 2026 05:06
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 27, 2026 05:08 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from b645aea to 337e5f5 Compare August 27, 2026 18:00
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 27, 2026 18:04 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from 337e5f5 to c21cf27 Compare August 27, 2026 18:38
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from e6f3f01 to 212e41c Compare August 28, 2026 23:49
@ayushiahjolia ayushiahjolia changed the title Parent durable OTel spans to shared execution trace feat(otel): add structured trace-context types and execution-trace resolver Aug 28, 2026
@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from 212e41c to 902d117 Compare August 28, 2026 23:57
@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from 902d117 to effdd0e Compare August 29, 2026 00:09
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 00:14 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 00:14 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the otel-shared-execution-trace branch from effdd0e to e3f2437 Compare August 29, 2026 01:17
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 01:21 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 01:21 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia marked this pull request as ready for review August 29, 2026 01:54
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 01:54 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 01:54 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 29, 2026 01:54 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

No actionable findings. Residual risk is limited to future integration of these currently unused trace-context types with the OTel plugins; this PR adds unit coverage only.

Reviewed commit e3f2437cf3751e147f6945757c114ddff26bd1ca. Workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

No actionable findings. This PR adds foundational OpenTelemetry trace-context types (Sampling, ExtractedContext, ExecutionTraceContext, canonical_trace_id, derive_execution_root_span_id) with no consumers yet, matching the PR description.

Verified as correct:

  • ID validity ranges match the OTel spec (128-bit trace IDs, 64-bit span IDs, all-zero rejected).
  • derive_execution_root_span_id is deterministic (blake2b, replay-stable), namespaced distinctly from the Workflow and operation span-ID derivations, and guards the all-zero case with or 1.
  • resolve trace-flag precedence (explicit SAMPLED/NOT_SAMPLED, else defer to root_sampled()) is consistent with its tests; the ... or 0 fallbacks in the complete-remote-parent and canonical_trace_id paths are unreachable given the validity guards and are harmless.
  • No import cycle is introduced; frozen dataclasses and complete type annotations satisfy the repo's typing conventions.
  • Test coverage is adequate for the new surface.

Residual test risk (not a defect, flagged for the follow-up PRs that wire these types):

  • When a complete remote parent is present but ExtractedContext.sampling is UNDECIDED, resolve re-samples via root_sampled() rather than honoring the incoming trace's own sampling decision. This is untested against real backend headers and could produce partial traces if the backend does not populate sampling alongside a joined remote parent — worth confirming when the extractor that builds ExtractedContext lands.
  • Minor: the module-level function canonical_trace_id is shadowed by the same-named int parameter of ExecutionTraceContext.resolve (execution_trace_context.py). No functional impact today, but a future edit to resolve that tries to call the function internally would silently bind the parameter instead; consider renaming one for clarity.

Reviewed commit e3f2437cf3751e147f6945757c114ddff26bd1ca. 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.

2 participants