Repository navigation
Conversation
…OTLP With SPACE_OTEL_EXPORTER_OTLP_ENDPOINT set, an API request that starts Temporal work now produces one connected trace: the API SERVER span, the workflow span, one span per activity attempt and nested child workflows, across the worker and both memory workers. - Workflow interceptor module (workflow-tracing.ts) registered by worker-tracing.ts in all three worker entrypoints, only when export is on. Spans leave the sandbox through a sink with callDuringReplay: false; span ids are derived from Temporal ids, so replay and retried workflow tasks never duplicate spans and the workflow random source is untouched. - Trace context travels in a `space-trace` Temporal header to activities, child workflows and continue-as-new runs. - Activity spans: one per attempt, child of the workflow span, with attempt, outcome, reasonCode and category. - roomAgentSupervisorWorkflow: one span per mission turn in the trace of the request that queued it, linked to the supervisor run. - Span links for continue-as-new runs and background continuations (roomAgentContinuation.sourceTraceparent, optional contract field). - Attribute allowlist with identifier-only values. - API: workflows get the request's own SERVER span id as parent; dotGoalWorkflow and API-started memory workflows get the memo. - Shared exporter (packages/runtime): optional span links, and a fix for a flush on an empty queue that stopped all later exports. - Integration test on the @temporalio/testing time-skipping server with a fake OTLP collector; replay test; docs updated. Closes oll4com#61 Signed-off-by: Joshua L. Lopez <joshualee.2733@gmail.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
Tracing can misreport failures and cancellations as completed and break trace continuity for interrupted-turn cleanup and background continuations, while supervisor replay coverage is missing.
5 open issues
What changed in this PR
This PR extends optional OTLP tracing across Temporal workflows and activity attempts, connecting worker execution to API request traces and addressing the coverage gaps in #61.
Changes:
- Adds workflow and mission spans, trace propagation, and continuation links across all three workers.
- Extends the shared exporter with span links and fixes exports stopping after an empty flush.
- Adds tracing, integration, and replay tests, plus configuration and coverage documentation.
| File | Description |
|---|---|
| packages/runtime/tests/otlp-span-exporter-shared.test.ts | Tests links, idle flush recovery, and queue bounds. |
| packages/runtime/src/otlp-span-exporter.ts | Supports span links and fixes empty-queue flushing. |
| packages/contracts/src/schemas.ts | Adds optional continuation source trace context. |
| package-lock.json | Locks Temporal dependencies and updates workspace metadata. |
| docs/quality/observability.md | Documents workflow tracing, links, and limitations. |
| docs/configuration-reference.md | Documents OTLP configuration across services. |
| apps/worker/tests/workflow-tracing.test.ts | Tests carriers, filtering, registration, and deduplication. |
| apps/worker/tests/support/fake-otlp-collector.ts | Provides a local test collector. |
| apps/worker/tests/room-continuation-fixture.ts | Supports traced continuation fixtures. |
| apps/worker/tests/room-background-continuation.test.ts | Tests continuation source-context preservation. |
| apps/worker/tests/otlp-workflow-tracing.integration.test.ts | Tests real workflow traces, links, and replay suppression. |
| apps/worker/tests/activity-observability.test.ts | Checks tracing registration across worker entrypoints. |
| apps/worker/src/workflows.ts | Wraps supervisor mission turns in tracing scopes. |
| apps/worker/src/workflow-tracing.ts | Implements workflow interceptors and mission scopes. |
| apps/worker/src/worker.ts | Registers shared tracing options. |
| apps/worker/src/worker-tracing.ts | Configures interceptors and replay-suppressed sinks. |
| apps/worker/src/trace-carrier.ts | Defines trace headers, deterministic IDs, and attribute filtering. |
| apps/worker/src/room-background-continuation.ts | Preserves source context for continuation links. |
| apps/worker/src/memory-mutation-worker.ts | Registers shared tracing options. |
| apps/worker/src/memory-maintenance-worker.ts | Registers shared tracing options. |
| apps/worker/src/activity-observability.ts | Parents activity spans using propagated workflow context. |
| apps/worker/package.json | Adds Temporal common and testing dependencies. |
| apps/api/tests/trace-context-workflow-parent.test.ts | Tests request-span parenting and memory workflow memos. |
| apps/api/src/trace-context.ts | Propagates the exported API span as parent. |
| apps/api/src/memory-mutation-coordinator.ts | Adds request trace context to workflow memos. |
| apps/api/src/memory-consolidation-coordinator.ts | Adds request trace context to workflow memos. |
| apps/api/src/dot-goals.ts | Passes request trace context when starting goals. |
Review effort: Balanced
Comment on lines
+128
to
+131
| const { traceparent: sourceRequestTraceparent, ...sourceTurn } = source.queueItem.turn; | ||
| const sourceTraceparent = sourceRequestTraceparent ?? source.queueItem.turn.roomAgentContinuation?.sourceTraceparent; | ||
| const item = roomAgentSupervisorQueueItemSchema.parse({ missionId: input.missionId, turn: { | ||
| ...source.queueItem.turn, | ||
| ...sourceTurn, |
Comment on lines
+102
to
+104
| const status = (result as { status?: unknown } | null)?.status; | ||
| if (status === "FAILED") return "FAILED_RESULT"; | ||
| return "COMPLETED"; |
| cancellationType: ChildWorkflowCancellationType.WAIT_CANCELLATION_COMPLETED | ||
| // One span per mission turn in the trace of the request that queued it | ||
| // (a no-op without OTLP export; it does no I/O and issues no commands). | ||
| await traceWorkflowScope({ name: "mission", traceparent: item.turn.traceparent, args: [item] }, async () => { |
| cancellationType: ChildWorkflowCancellationType.WAIT_CANCELLATION_COMPLETED | ||
| // One span per mission turn in the trace of the request that queued it | ||
| // (a no-op without OTLP export; it does no I/O and issues no commands). | ||
| await traceWorkflowScope({ name: "mission", traceparent: item.turn.traceparent, args: [item] }, async () => { |
Comment on lines
+266
to
+270
| const cases: Array<[string, string]> = [ | ||
| [buildCodexAppServerTurnWorkflowId(turn("a")), "workflows.ts"], | ||
| ["space-memory-maintenance:tracing", "memory-maintenance-workflows.ts"], | ||
| ["space-memory-consolidation:tracing-1", "memory-maintenance-workflows.ts"] | ||
| ]; |
Owner
|
Thank you for the time and care you put into this pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #61
Problem and user impact
With
SPACE_OTEL_EXPORTER_OTLP_ENDPOINTset, only the API exported spans. Workers emitted none, so a slow or failing turn, Room Agent mission or memory repair could only be followed by joining log lines by hand, and background continuations started a new, unlinked trace (OBS-001 / OBS-007).Change
One connected trace from the API SERVER span to the workflow span to one span per activity attempt, across
worker.ts,memory-maintenance-worker.tsandmemory-mutation-worker.ts.packages/runtime(no copy): bounded queue with counted drops, per-POST timeout, never blocks workflow/activity work. Added optional span links, and fixed a flush on an empty queue that stopped all later exports. API behavior andapps/api/tests/otlp-span-exporter.test.tsunchanged.worker-tracing.tsregisters the workflow interceptor module (workflow-tracing.ts) and activity interceptors in all three entrypoints only when the endpoint is set; with it unset nothing is registered.callDuringReplay: false. Span ids are derived from Temporal ids, so replay and retried workflow tasks never duplicate spans and the workflow random source is untouched.traceparentspan id (the API now passes its own SERVER span id); one span per activity attempt with attempt, outcome,reasonCode/category; trace context travels in aspace-traceTemporal header to activities, child workflows (memoryMaintenanceWorkflow→memoryConsolidationWorkflow) and continue-as-new runs.roomAgentSupervisorWorkflow: one span per mission turn in the trace of the request that queued it, linked to the supervisor run.dotGoalWorkflowand API-started memory workflows get the memo.roomAgentContinuation.sourceTraceparent, optional contract field).@temporalio/testingis added as a worker devDependency for the integration test (see [$400] Test every Temporal workflow on a real time-skipping test server and gate replay determinism in CI #60).docs/quality/observability.mdanddocs/configuration-reference.mdupdated, including the "Span export covers API requests only" bullet.Verification
Linux x86_64 (kernel 6.12), Node v22.21.1, npm 10.9.4, commit 519da28 (this PR's head). Every line below was actually run; nothing on macOS or Windows.
npm run checknpm test -w @space/workersqlite3CLI: 250/251, the one failure iscodex-cli-completion.test.ts(spawn sqlite3 ENOENT), which is unchanged by this PR and fails the same way onmaintherenpm test -w @space/api -- otlp@space/apitrace-context testsnpm test -w @space/runtimenpm test -w @space/contractsotlp-workflow-tracing.integration.test.ts,workflow-tracing.test.ts) on the@temporalio/testingtime-skipping server with a local fake collectorcallDuringReplay: truenpm run lintnpm run hygiene:preflightnpm run public:export:verifyok: trueJaeger (manual check). Real
jaegertracing/all-in-onewithSPACE_OTEL_EXPORTER_OTLP_ENDPOINT=http://127.0.0.1:4318. The run used a local, uncommitted demo test: a Fastify demo route wired with the API's realcreateHttpObservabilityandTemporalCodexAppServerTurnStarter, plus the real workers' tracing options, running the real workflows on the time-skipping Temporal server with stub activities (no AI provider). It is not a run of a full installed stack.Turn trace (API →
codexAppServerTurnWorkflow→ two attempts ofrunCodexAppServerTurn, first FAILED withCODEX_APP_SERVER_UNAVAILABLE, second COMPLETED):Memory trace (API →
memoryMaintenanceWorkflow→ activities → childmemoryConsolidationWorkflow→ activities, across the maintenance and mutation workers):Span listing: https://raw.githubusercontent.com/DocGrok1/spaceapp/e139be4701fd36c42499b2252364296a10b185b7/jaeger-trace-summary.txt
The demo's stub activity deliberately threw an error containing prompt text, a URL with a token and a file path; none of it appears in the exported attributes.
Signed-off-by:line (DCO 1.1); MIT License.AI-assisted; the results above are from commands actually executed. The "Verify export provenance" step is expected to fail on this fork PR, per the issue.