Commit 092361e
* feat(observability): correlation envelope across orchestrator logs, TaskEvents, agent OTel (#245)
Make the {task_id, user_id, repo, trace_id} correlation envelope observable on
every plane so a task's actions join across CloudWatch logs, TaskEvents, and the
X-Ray trace without manual stitching. Builds on #515, which already persists
otel_trace_id on the TaskRecord (the orchestrator<->agent bridge).
Phase 0 — contract:
- docs/design/OBSERVABILITY.md: "Correlation envelope" section — field table
(repo/trace_id/session_id nullable), shared snake_case naming with #215/#237/
#249, and the join model (TaskRecord bridges the two planes; no single W3C
root since the orchestrator runs before the agent trace exists). Starlight
mirror synced.
Phase 1 — orchestrator (TS):
- logger.ts: add logger.child(context) merging a persistent envelope into every
line; per-call data wins on collision.
- orchestrate-task.ts + shared/orchestrator.ts: derive a child logger in the
lifecycle functions (orchestrate handler, finalizeTask, hydrateAndTransition,
failTask) so admission->terminal log lines carry {task_id, user_id, repo}.
- emitTaskEvent gains an optional EventCorrelation param; lifecycle events now
stamp user_id/repo as top-level fields (repo omitted for repo-less workflows).
Phase 1 — agent (Python):
- progress_writer.py: _ProgressWriter takes user_id/repo; _put_event stamps
{user_id, repo, trace_id} (trace_id read per-event from the active span) so
the agent event stream joins to X-Ray. Optional fields omitted, never null.
- observability.py: set_session_id propagates user.id/repo.url baggage, guarding
each field (empty/None not stamped).
- pipeline.py: root task.pipeline span gains user.id; both writer sites pass the
envelope. server.py: propagate even when session_id is empty.
Phase 2 needs no new code: #515's TaskRecord.otel_trace_id plus the agent event
trace_id stamping give the cross-plane join. Both _ProgressWriters are built
inside an active span, so trace_id is always valid.
Tests: logger child-merge (4), orchestrate-task envelope incl. repo-less (3),
progress_writer envelope incl. omit-not-null (4), set_session_id incl. empty
session (3). No #209 SessionRole regression (user_id is read-only here).
CDK 2236 pass, agent 1132 pass; eslint/ruff/docs-sync clean.
* fix(#245): address PR #557 review — trace-read fail-open, repo type, coverage
Review (4 plugin agents) found no blocking issues; addressed the three
substantive items:
- silent-failure (MEDIUM): current_otel_trace_id() is read inside
progress_writer._put_event's DDB try-block. If a misconfigured tracer raised,
the error was caught by the DDB handler, misclassified as "unknown", and
tripped the SHARED progress circuit breaker — silencing the whole event
stream (turns, milestones, #251 agent_blocked) over a tracing fault. Make
current_otel_trace_id() never raise: a tracer error degrades to None (its
documented graceful-missing contract), so the trace_id field is simply
omitted. Fixes every caller, incl. #515's crash-path persistence.
- type coherence: narrow EventCorrelation.repo and failTask's repo param from
`string | null` to `string | undefined`, matching the source TaskRecord.repo
(never null). Removes the redundant third "absent" state; consumers already
omit-on-falsy so behavior is unchanged. tsc + eslint clean.
- test coverage: add the highest-value missing test — _run_task_background
reaches set_session_id with the envelope on the empty-session/known-user
branch (the widened trigger the feature depends on). Add a defensive test
that a throwing tracer degrades current_otel_trace_id() to None.
Skipped (churn > value, behavior already correct): failTask→EventCorrelation
refactor, Python str=""-vs-None normalization, level/message collision guard.
agent 1182 pass, CDK orchestrate-task+logger 107 pass; ruff/eslint/tsc clean.
* style(#245): ruff-format new test files (fix CI mutation)
CI 'Fail build on mutation' tripped: ruff format rewrote the backslash
line-continuation `with` statements in the two new test files into the
parenthesized multi-context-manager form. Apply the formatter locally and
commit, matching the repo convention. No behavior change.
* feat(#245): surface correlation envelope in event API + close review nits
Independent review follow-ups (all non-blocking) folded into this PR:
1. API passthrough (#1): the {user_id, repo, trace_id} envelope was stamped on
TaskEvents in DDB but stripped by get-task-events.ts and get-task-replay.ts,
so bgagent watch/events/replay never surfaced per-event correlation. Add the
three optional fields to EventRecord + ReplayEvent (cdk) and TaskEvent +
ReplayEvent (cli mirror), and pass them through in both handlers (omit-when-
absent). API_CONTRACT.md documents the per-event envelope; types-sync clean.
2. Coverage docs (#2): OBSERVABILITY.md now notes task_created predates the
envelope and joins by task_id alone, and that CLI consumers can join at the
task level via TaskRecord.otel_trace_id.
3. Doc wording (#3): correlation-envelope table says repo is "absent (key
omitted, not null/'')" for repo-less workflows, matching the implementation
(#248) instead of the earlier "null".
4. Consistency (#4): failTask's transition-failure log and loadBlueprintConfig
now use logger.child({task_id,user_id?,repo?}) like the rest of the lifecycle
code, instead of base logger + inlined fields.
Tests: direct emitTaskEvent(..., correlation) unit tests (stamp / repo-less
omit / back-compat no-envelope); get-task-events + get-task-replay passthrough
incl. task_created omission.
CDK 2256 pass + synth clean; CLI 578 pass; agent 1182 pass; eslint/ruff/tsc/
types-sync/docs-sync clean.
* refactor(#245): address review — rename, envelopeFor helper, loadBlueprintConfig user_id
Second independent review (ayushtr-aws, approve-with-minor): all 6 minor items.
1. Rename set_session_id → propagate_correlation_context (observability.py): it
propagates the full envelope, not just session id, and is called with an
empty session_id — the old name misled. Updated the sole caller (server.py)
and tests.
2. Drop the stray "// ponytail:" marker prefix in logger.ts (kept the content).
3. loadBlueprintConfig's child logger now carries user_id (was task_id/repo
only) — its log lines are admission→terminal and must join by user_id per AC.
4. Extract envelopeFor(task) → { log, correlation } in orchestrator.ts, the
single source for the log child and the event correlation. Replaces the
hand-rolled logger.child + EventCorrelation literal duplicated across 5 sites
(orchestrate-task, hydrateAndTransition, finalizeTask, failTask,
loadBlueprintConfig) — the drift that produced item 3.
5. Comment on the Python ""-vs-None param asymmetry in
propagate_correlation_context (mirrors upstream config types).
6. set-compare baggage keys in the observability test instead of exact order
(order isn't part of the contract).
Tests: envelopeFor unit tests (log child carries user_id incl. repo-less
omission); observability/server tests updated for the rename.
CDK 2258 pass + synth clean; agent 1182 pass; eslint/ruff/ruff-format/tsc clean.
---------
Co-authored-by: bgagent <bgagent@noreply.github.com>
1 parent 9467a94 commit 092361e
23 files changed
Lines changed: 654 additions & 122 deletions
File tree
- agent
- src
- tests
- cdk
- src/handlers
- test/handlers
- cli/src
- docs
- design
- src/content/docs/architecture
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
67 | 72 | | |
68 | | - | |
69 | | - | |
70 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
71 | 86 | | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | 87 | | |
78 | 88 | | |
79 | | - | |
80 | | - | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
81 | 105 | | |
82 | 106 | | |
83 | 107 | | |
84 | 108 | | |
85 | 109 | | |
86 | | - | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
87 | 117 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
679 | 679 | | |
680 | 680 | | |
681 | 681 | | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
682 | 685 | | |
683 | 686 | | |
684 | 687 | | |
685 | 688 | | |
686 | 689 | | |
687 | 690 | | |
688 | | - | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
689 | 694 | | |
690 | 695 | | |
691 | 696 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
| 42 | + | |
41 | 43 | | |
42 | 44 | | |
43 | 45 | | |
| |||
373 | 375 | | |
374 | 376 | | |
375 | 377 | | |
376 | | - | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
377 | 385 | | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
378 | 391 | | |
379 | 392 | | |
380 | 393 | | |
| |||
467 | 480 | | |
468 | 481 | | |
469 | 482 | | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
470 | 488 | | |
471 | 489 | | |
472 | 490 | | |
| |||
478 | 496 | | |
479 | 497 | | |
480 | 498 | | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
481 | 505 | | |
482 | 506 | | |
483 | 507 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
483 | 483 | | |
484 | 484 | | |
485 | 485 | | |
486 | | - | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
487 | 492 | | |
488 | 493 | | |
489 | 494 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| |||
452 | 452 | | |
453 | 453 | | |
454 | 454 | | |
455 | | - | |
456 | | - | |
457 | | - | |
458 | | - | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
459 | 461 | | |
460 | 462 | | |
461 | 463 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
267 | 267 | | |
268 | 268 | | |
269 | 269 | | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
270 | 323 | | |
271 | 324 | | |
272 | 325 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
291 | 326 | | |
292 | 327 | | |
293 | 328 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
219 | 219 | | |
220 | 220 | | |
221 | 221 | | |
222 | | - | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
223 | 226 | | |
224 | 227 | | |
225 | 228 | | |
226 | 229 | | |
227 | 230 | | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
228 | 234 | | |
229 | 235 | | |
230 | 236 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
200 | 204 | | |
201 | 205 | | |
202 | 206 | | |
| |||
0 commit comments