Repository navigation
feat(workflow): trace step attempts with OpenTelemetry - #7635
Conversation
reflex-workflow had no OpenTelemetry code: a run started from a traced event handler produced steps invisible to the trace, anything a step traced itself started an unrelated trace, and every step task inherited whatever context was current when run_workflows was entered, so a worker started inside a span parented every later step under that long-ended span. Add reflex_workflow.otel on the reflex_base.otel pattern: an `enabled` flag the engine checks at each trace point, with the opentelemetry imports confined to enable(), so the package keeps its SQLAlchemy-only dependency list and imports nothing from opentelemetry until instrumented. ReflexInstrumentor enables and disables it alongside the framework trace points when reflex-workflow is importable. Each step attempt gets a CONSUMER span named `<table>.<step>`, started from an explicit empty parent context so it is a trace root whatever the worker inherited. It links to the span that scheduled it rather than nesting under it, since a step may run days after wake_in, on another worker after a lease expiry, or as one of many fanned-out children. Call.encode() stamps the current W3C traceparent into the payload under a reserved key of next_args, next to the claim's event key, so start, run, deliver, chaining, schedules and fan-out all carry it without a schema change; execute() strips it before the step sees its arguments, and a retry re-stamps the failed attempt's span so retries chain. Attributes are the table, step, attempt number, lane and outcome, plus error.type and the recorded exception on a failed attempt. Arguments and row columns are never recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjgvPv7AqSycFLeXwSfB6D
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
[Medium impact] The PR appears safe to merge; no new actionable issues remain. SummaryAdds OpenTelemetry spans for workflow step attempts.
No new actionable issues were found. The previous unnumbered finding is fixed: Reviews (3) · Last reviewed commit: "refactor(otel): tidy workflow trace-poin..." · Reviewed by Greptile |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjgvPv7AqSycFLeXwSfB6D
wake_on_schedule turns the exception into the row's last_error, so the span ended as failed with no exception or error.type on it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjgvPv7AqSycFLeXwSfB6D
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjgvPv7AqSycFLeXwSfB6D
Merging this PR will not alter performance1 warning
Comparing |
Merge the doubled TYPE_CHECKING block in reflex_workflow.otel, drop the stale/missing outcomes from record_outcome's docs (execute returns them before the span opens), make run_attempt's arguments keyword-only, and toggle the workflow trace points through one helper so _instrument keeps its complexity.
Requested by Masen · project thread
First PR for ENG-13483 (plan items A, B, C).
Type of change
What and why
reflex-workflow had no OpenTelemetry code. Checked on main with the SDK, an in-memory exporter and
ReflexInstrumentor().instrument():run_workflowswas entered (asyncio.create_taskcopies contextvars for the runner loop and each step), so a worker entered inside a span parented every later step under that long-ended span.This PR:
reflex_workflow.otel, on thereflex_base.otelpattern: anenabledflag the engine checks at each trace point,opentelemetryimports confined toenable(). reflex-workflow keeps its SQLAlchemy-only dependency list and imports nothing fromopentelemetryuntil instrumented (a test pins that).ReflexInstrumentorenables and disables the workflow trace points alongside the framework's whenreflex_workflow.otelis importable, so oneinstrument()call, oropentelemetry-instrument, covers both.CONSUMERspan per step attempt, named<table>.<step>, started from an explicit empty parent context so it is a trace root whatever the worker inherited. It links to the span that scheduled it rather than nesting under it: a step may run days afterwake_in, on another worker after a lease expiry, or as one of many fanned-out children. Attributes:reflex.workflow.table,.step,.attempt,.lane,.outcome;error.typeand the recorded exception on a failed attempt (statusERRORforretry/failed). Step arguments and row columns are never recorded.Call.encode()stamps the current W3Ctraceparentinto the payload under a reserved top-level key ofnext_args, next to the claim's existingevent_key, sostart,run,deliver, chaining, schedules andfan_outall carry it.execute()strips it before the step sees its arguments; a retry re-stamps the failed attempt's span, so retries chain attempt → attempt.Not in this PR (later items on the ticket): suppressing the engine's bookkeeping queries from DB instrumentations, and worker metrics.
Tests
tests/units/reflex_workflow/test_otel.py(no Postgres): disabled path imports noopentelemetryand changes nothing;stamp/encodecarry the current span; the step span is a root even when entered under another span, with the link, kind and attributes; unusabletraceparentvalues yield no link; failed attempts record the exception and status.test_runner.py::test_steps_are_traced_as_linked_root_spans(Postgres): a chain started under a handler span linksfirst→ handler andsecond→first; a flaky step's three attempts areretry,retry,ok, each linked to the previous attempt;next_argsis clean afterwards.tests/units/reflex_otel/test_init.py: the instrumentor toggles the workflow trace points.Ran locally: full
tests/units/reflex_workflowagainst Postgres 16 plustests/units/reflex_otelandtests/units/reflex_base/test_otel.py(1122 passed),ruff check/format,pyrighton the touched packages and tests, codespell.Generated by Claude Code