Define correlation-first OpenTelemetry observability for workshop runs (5.1.1)#63
Define correlation-first OpenTelemetry observability for workshop runs (5.1.1)#63lodyai[bot] wants to merge 10 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
Runtime instrumentation and the WalkthroughChangesWorkflow observability contract
Suggested labels: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 3 warnings)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Record the decision to export workshop telemetry from a host-side bridge that translates a run directory's `events.jsonl` into OTLP traces and metrics. In-workflow instrumentation is rejected because the ODW dialect bans runtime imports and temporal calls, and a stock collector `filelog` receiver cannot reconstruct span parentage.
Introduce step 5.1 (OpenTelemetry export for workshop runs) with tasks for the trace bridge (5.1.1), workshop metrics (5.1.2), and operator documentation plus a collector smoke test (5.1.3), per ADR 003.
Plan the delivery of `scripts/odw-otel-bridge` in five milestones: schema characterization from a recorded run, red fixture tests, the pure event parser and span builder, the OTLP exporter and entry point, and live-tail mode with documentation. The plan is DRAFT and awaits approval before implementation.
Rewrite ADR 003, roadmap phase 5, and the 5.1.1 ExecPlan around the source design conversation, which rejects post-hoc `events.jsonl` correlation as unsound under parallelism and retries. The adopted architecture: ODW mints an opaque agent invocation id at dispatch, Claude Code inherits it through W3C trace context, Codex carries it in per-invocation OTLP exporter headers, and a local `workflow-telemetryd` collector stores raw OTLP records beside a versioned relational projection in SQLite under the `.workshop` sidecar. Identity uses three distinct keys (correlation id, workflow invocation id, node attempt id) with a workflow-neutral `leynos.*` attribute layer and a versioned observability-context envelope so sibling workflows such as Dakar join the same fabric through explicit invocation edges. Roadmap phase 5 now sequences the work as contract and collector (5.1), ODW correlation substrate (5.2), workflow and provider instrumentation (5.3), operator query surfaces (5.4), and cross-workflow correlation through Dakar (5.5). Task 5.1.1 is re-scoped from building an events bridge to defining the contract; the ExecPlan is rewritten accordingly, with the bridge retained only as a `heuristic`-confidence import path for historical runs.
List the correlation-first observability decision record under decision records and the workflow observability contract plan under execution plans, keeping the index complete after rebasing onto the new contents document.
Write `docs/workflow-observability-contract.md`, the normative version-1 contract that ADR 003's fabric depends on: the identity model, the four naming domains, the `leynos.*` attribute registry, span topology, the `WorkflowObservabilityContextV1` envelope and its collector-endpoint sink rule, correlation headers and collector validation duties, telemetry binding records with the source-to-confidence mapping, the logical node key grammar, the metrics cardinality policy, and the provisional ODW agent-event extensions. This is milestone 1 of the ExecPlan; the schemas, fixtures, and validation tests follow. Record the test-location, naming-domain, and fixture-layout decisions in the plan's decision log and mark it in progress.
Add `tests/modules/observability-contract.test.ts` and the consolidated `tests/fixtures/observability-contract/fixtures.json`, plus `ajv` (JSON Schema draft 2020-12) as a dev dependency. The suite validates envelope, binding, and agent-event fixtures against the schemas under `schemas/observability/` and checks the logical node key grammar. This is the red step: the schema files do not exist yet, so the suite fails with `ENOENT` on the envelope schema. Milestone 3 adds the schemas to turn it green.
Add the three JSON Schema draft 2020-12 documents under `schemas/observability/`: the `WorkflowObservabilityContextV1` envelope (closed object, const `schemaVersion`, `otlp+http(s)` sink scheme, environment-only credential references), the telemetry binding record (snake_case, dotted `binding_type`, and a source-to-confidence conditional that forces `exact` for trace-context and header sources and `heuristic` for the events-jsonl import path), and the provisional agent-event extensions. Carry nanosecond timestamps as decimal strings rather than JSON numbers to avoid the 2^53 precision limit, and note this in the contract. Enable `allowUnionTypes` on the strict validator for the scalar attribute union. The suite now passes 31/31.
Link `docs/workflow-observability-contract.md` from ADR 003's migration plan, the developers' guide normative references, and the documentation contents index, and tick roadmap task 5.1.1 with pointers to the delivered document, schemas, and test. Update the ExecPlan's living sections with the milestone outcomes, the nanosecond-string and union-type discoveries, and the retrospective.
Set the ExecPlan status to COMPLETE: all four milestones are done, the full `make all` gate passes, and a CodeRabbit review returned zero findings.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/execplans/5-1-1-otel-support.md`:
- Around line 237-242: Update all references to the observability contract test
in the Milestone 2 plan from tests/observability-contract.test.ts to
tests/modules/observability-contract.test.ts, including the test command,
expected-red transcript, and acceptance text. Keep the fixture cases and
documented gated-test location consistent throughout.
In `@schemas/observability/workflow-observability-context.v1.json`:
- Around line 69-72: Update the endpoint pattern in
schemas/observability/workflow-observability-context.v1.json at lines 69-72 to
continue accepting only otlp+http(s) URLs while rejecting any URI userinfo
credentials before the host. Add a negative credentialed-endpoint fixture in
tests/fixtures/observability-contract/fixtures.json at lines 116-137, ensuring
validation confirms endpoints containing user:password@ are rejected.
- Around line 37-40: Update the traceparent pattern in
schemas/observability/workflow-observability-context.v1.json to enforce W3C
validity by rejecting all-zero trace IDs, all-zero span IDs, and the reserved ff
version while retaining the existing lowercase hex and width requirements. Add
fixtures covering each invalid case in
tests/fixtures/observability-contract/fixtures.json alongside the existing
malformed traceparent example.
- Around line 16-29: Replace the existing non-empty string validation for
workflowInvocationId and nodeAttemptId in
schemas/observability/workflow-observability-context.v1.json,
agent_invocation_id and agent_process_id in
schemas/observability/agent-event-extensions.v1.json, and the corresponding IDs
in schemas/observability/telemetry-binding.v1.json with one shared UUIDv7 schema
shape. Add malformed-ID fixtures covering the envelope, binding, and agent-event
cases in tests/fixtures/observability-contract/fixtures.json, ensuring they are
rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9d530be7-d07a-4e5a-bce5-f123286a430b
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
docs/adr-003-opentelemetry-observability.mddocs/contents.mddocs/developers-guide.mddocs/execplans/5-1-1-otel-support.mddocs/roadmap.mddocs/workflow-observability-contract.mdpackage.jsonschemas/observability/agent-event-extensions.v1.jsonschemas/observability/telemetry-binding.v1.jsonschemas/observability/workflow-observability-context.v1.jsontests/fixtures/observability-contract/fixtures.jsontests/modules/observability-contract.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/nixie(auto-detected)
| Milestone 2 (red): add `tests/observability-contract.test.ts` with | ||
| fixture-driven cases before the schemas exist: valid envelopes (full parent | ||
| context; minimal correlation-only; sink absent), invalid envelopes (missing | ||
| `schemaVersion`, inline credential material in `sink`, database-path sink, | ||
| unknown `confidence` value), and logical-node-key strings that must match or | ||
| fail a documented pattern. Run the suite and record the expected failures. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the stale top-level test path.
Lines 237, 261, and 272 still reference
tests/observability-contract.test.ts, but Line 144 records that the gated
test lives at tests/modules/observability-contract.test.ts. Update the
plan, commands, expected-red transcript, and acceptance text together.
As per coding guidelines, “Keep documentation accurate and current”.
Also applies to: 260-263, 270-280
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/execplans/5-1-1-otel-support.md` around lines 237 - 242, Update all
references to the observability contract test in the Milestone 2 plan from
tests/observability-contract.test.ts to
tests/modules/observability-contract.test.ts, including the test command,
expected-red transcript, and acceptance text. Keep the fixture cases and
documented gated-test location consistent throughout.
Source: Coding guidelines
| "workflowInvocationId": { | ||
| "$ref": "#/$defs/identifier" | ||
| }, | ||
| "parent": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["workflowInvocationId", "nodeAttemptId"], | ||
| "properties": { | ||
| "workflowInvocationId": { | ||
| "$ref": "#/$defs/identifier" | ||
| }, | ||
| "nodeAttemptId": { | ||
| "$ref": "#/$defs/identifier" | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File list\n'
git ls-files 'schemas/observability/*.json' 'tests/fixtures/observability-contract/fixtures.json' | sort
printf '\n## Search for uuid-related definitions/usages\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' -i 'uuidv7|uuid|ulid|pattern|format' schemas tests | sed -n '1,240p'
printf '\n## Key schema excerpts\n'
for f in \
schemas/observability/workflow-observability-context.v1.json \
schemas/observability/agent-event-extensions.v1.json \
schemas/observability/telemetry-binding.v1.json \
tests/fixtures/observability-contract/fixtures.json
do
echo "\n### $f"
nl -ba "$f" | sed -n '1,220p'
doneRepository: leynos/df12-build
Length of output: 7499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
schemas/observability/workflow-observability-context.v1.json \
schemas/observability/agent-event-extensions.v1.json \
schemas/observability/telemetry-binding.v1.json
do
echo "### $f"
rg -n '"\$defs"|identifier|workflowInvocationId|nodeAttemptId|agent_invocation_id|agent_process_id' "$f"
echo
doneRepository: leynos/df12-build
Length of output: 1116
🔍 Additional research query:
Deepwiki: leynos/df12-build observability schema UUIDv7 identifier definitions in schemas/observability/workflow-observability-context.v1.json agent-event-extensions.v1.json telemetry-binding.v1.json
💡 Result:
No relevant context found from external knowledge sources.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
schemas/observability/workflow-observability-context.v1.json \
schemas/observability/agent-event-extensions.v1.json \
schemas/observability/telemetry-binding.v1.json \
tests/fixtures/observability-contract/fixtures.json
do
echo "### $f"
sed -n '1,140p' "$f" | cat -n
echo
doneRepository: leynos/df12-build
Length of output: 12932
Enforce UUIDv7 for execution IDs.
Replace the current minLength: 1 rules for workflowInvocationId, nodeAttemptId, agent_invocation_id, and agent_process_id with a shared UUIDv7 shape, and add malformed-ID fixtures for the envelope, binding, and agent-event cases.
📍 Affects 4 files
schemas/observability/workflow-observability-context.v1.json#L16-L29(this comment)schemas/observability/agent-event-extensions.v1.json#L20-L30schemas/observability/telemetry-binding.v1.json#L26-L32tests/fixtures/observability-contract/fixtures.json#L65-L176tests/fixtures/observability-contract/fixtures.json#L221-L331
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/observability/workflow-observability-context.v1.json` around lines 16
- 29, Replace the existing non-empty string validation for workflowInvocationId
and nodeAttemptId in
schemas/observability/workflow-observability-context.v1.json,
agent_invocation_id and agent_process_id in
schemas/observability/agent-event-extensions.v1.json, and the corresponding IDs
in schemas/observability/telemetry-binding.v1.json with one shared UUIDv7 schema
shape. Add malformed-ID fixtures covering the envelope, binding, and agent-event
cases in tests/fixtures/observability-contract/fixtures.json, ensuring they are
rejected.
Source: Learnings
| "traceparent": { | ||
| "type": "string", | ||
| "pattern": "^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== schema snippet ==\n'
cat -n schemas/observability/workflow-observability-context.v1.json | sed -n '25,60p'
printf '\n== fixtures snippet ==\n'
cat -n tests/fixtures/observability-contract/fixtures.json | sed -n '145,190p'
printf '\n== search traceparent references ==\n'
rg -n "traceparent|trace parent|w3c" schemas tests workflows . || trueRepository: leynos/df12-build
Length of output: 4625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== schema consumers ==\n'
rg -n "workflow-observability-context\.v1\.json|workflow-observability-context" . || true
printf '\n== JSON schema validation code ==\n'
rg -n "jsonschema|ajv|validate.*schema|schema.*validate|traceparent" . || true
printf '\n== contract docs excerpt ==\n'
cat -n docs/workflow-observability-contract.md | sed -n '180,220p'Repository: leynos/df12-build
Length of output: 5718
Tighten the traceparent pattern to enforce W3C validity. Reject all-zero trace IDs/span IDs and the reserved ff version; the current regex only checks hex width and casing, so invalid parent contexts can still pass through. Add fixtures for those invalid cases alongside the existing malformed example.
📍 Affects 2 files
schemas/observability/workflow-observability-context.v1.json#L37-L40(this comment)tests/fixtures/observability-contract/fixtures.json#L158-L166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/observability/workflow-observability-context.v1.json` around lines 37
- 40, Update the traceparent pattern in
schemas/observability/workflow-observability-context.v1.json to enforce W3C
validity by rejecting all-zero trace IDs, all-zero span IDs, and the reserved ff
version while retaining the existing lowercase hex and width requirements. Add
fixtures covering each invalid case in
tests/fixtures/observability-contract/fixtures.json alongside the existing
malformed traceparent example.
| "endpoint": { | ||
| "type": "string", | ||
| "pattern": "^otlp\\+https?://" | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- schema excerpt ---'
nl -ba schemas/observability/workflow-observability-context.v1.json | sed -n '1,160p'
echo
echo '--- fixture excerpt ---'
nl -ba tests/fixtures/observability-contract/fixtures.json | sed -n '90,170p'
echo
echo '--- search for endpoint validation and inline credentials ---'
rg -n '"endpoint"|userinfo|credentials|otlp\+http|otlp\+https|pattern' schemas tests -SRepository: leynos/df12-build
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path, start, end in [
("schemas/observability/workflow-observability-context.v1.json", 1, 180),
("tests/fixtures/observability-contract/fixtures.json", 90, 170),
]:
print(f"--- {path} ---")
lines = Path(path).read_text().splitlines()
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:5d} {lines[i-1]}")
print()
PY
echo '--- search ---'
python3 - <<'PY'
from pathlib import Path
import re
paths = [
Path("schemas/observability/workflow-observability-context.v1.json"),
Path("tests/fixtures/observability-contract/fixtures.json"),
]
patterns = [r'userinfo', r'credentials', r'otlp\+http', r'otlp\+https', r'"endpoint"', r'pattern']
for path in paths:
print(f"=== {path} ===")
text = path.read_text()
for pat in patterns:
if re.search(pat, text):
print(f"matched: {pat}")
PYRepository: leynos/df12-build
Length of output: 6573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
paths = [
"docs/workflow-observability-contract.md",
"schemas/observability/workflow-observability-context.v1.json",
"tests/fixtures/observability-contract/fixtures.json",
]
for path in paths:
p = Path(path)
if p.exists():
print(f"--- {path} ---")
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if re.search(r'endpoint|userinfo|credential|authRef|token|secret|otlp\+http|otlp\+https', line, re.I):
lo = max(1, i - 3)
hi = min(len(text), i + 3)
for j in range(lo, hi + 1):
print(f"{j:5d} {text[j-1]}")
print()
PY
echo '--- repo-wide search ---'
python3 - <<'PY'
from pathlib import Path
import re
needles = [r'sink\.endpoint', r'userinfo', r'authRef', r'inline credential', r'otlp\+http://', r'otlp\+https://']
for path in Path('.').rglob('*'):
if path.is_file() and path.suffix in {'.md', '.json', '.js', '.ts', '.py', '.yml', '.yaml'}:
try:
text = path.read_text()
except Exception:
continue
hits = [n for n in needles if re.search(n, text, re.I)]
if hits:
print(path)
for h in hits:
print(f" matched: {h}")
PYRepository: leynos/df12-build
Length of output: 8889
Reject inline credentials in sink.endpoint.
schemas/observability/workflow-observability-context.v1.json#L69-L72 only checks the otlp+http(s):// prefix, so otlp+http://user:secret@collector.example still slips through and can persist secrets in run artefacts. Tighten the pattern to exclude URI userinfo, and add a negative fixture for a credentialed endpoint in tests/fixtures/observability-contract/fixtures.json.
📍 Affects 2 files
schemas/observability/workflow-observability-context.v1.json#L69-L72(this comment)tests/fixtures/observability-contract/fixtures.json#L116-L137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/observability/workflow-observability-context.v1.json` around lines 69
- 72, Update the endpoint pattern in
schemas/observability/workflow-observability-context.v1.json at lines 69-72 to
continue accepting only otlp+http(s) URLs while rejecting any URI userinfo
credentials before the host. Add a negative credentialed-endpoint fixture in
tests/fixtures/observability-contract/fixtures.json at lines 116-137, ensuring
validation confirms endpoints containing user:password@ are rejected.
Summary
This branch designs the correlation-first OpenTelemetry (OTel) observability
fabric for df12-build and delivers the first roadmap task on it, 5.1.1: the
version-1 workflow observability contract. The design (ADR 003) puts identity
minting in the Open Dynamic Workflows (ODW) host at dispatch, has Claude Code
inherit it through W3C trace context and Codex carry it in per-invocation
OTLP exporter headers, and lands native provider telemetry plus a versioned
relational projection in SQLite via a local
workflow-telemetrydcollector,under the
.workshopsidecar. The contract is workflow-neutral (leynos.*identity attributes and a versioned observability-context envelope) so
sibling workflows such as leynos/dakar
join through explicit invocation edges. Post-hoc
events.jsonlcorrelationis rejected as the primary mechanism (it binds telemetry to the wrong nodes
under parallelism and retries) and kept only as a
heuristic-confidenceimport path.
Task 5.1.1 is the narrow waist the rest of phase 5 depends on. It is now
implemented as a normative contract document, three JSON Schemas, and a
validation suite. No collector or ODW instrumentation runtime code lands
here; those are later tasks (5.1.2, 5.2, 5.3).
Roadmap task: (5.1.1)
Execplan (COMPLETE):
docs/execplans/5-1-1-otel-support.md
Review walkthrough
docs/adr-003-opentelemetry-observability.md
for the governing decision: identity model, provider asymmetry, the
collector and two-layer SQLite store, attribute namespaces, and the
cross-workflow envelope.
docs/workflow-observability-contract.md:
14 numbered sections fixing the identity model, the four naming domains,
the
leynos.*attribute registry, span topology, theWorkflowObservabilityContextV1envelope and its collector-endpoint sinkrule, correlation headers and collector duties, telemetry bindings with
the source-to-confidence mapping, the logical node key grammar, the metrics
cardinality policy, and the provisional ODW agent-event extensions.
schemas/observability/
(three draft 2020-12 schemas) and
tests/modules/observability-contract.test.ts,
which validates the fixtures against them and checks the node-key grammar
(31 tests).
docs/roadmap.md
sequences the remaining work: collector (5.1.2), ODW substrate (5.2),
instrumentation (5.3), query surfaces (5.4), and Dakar correlation (5.5).
Validation
make all: pass (check-fmt, lint, typecheck, markdownlint + spelling,nixie, docstring-coverage, tests, workflow-freshness, Dafny
verify-modules). Tests: 397 module + 124 node + 21 script.
coderabbit review --agent: zero findings across all 13 changed files.Notes
numbers lose precision above 2^53) and documents this with rationale; the
collector step must honour it.
confirm the field names against what ODW can actually emit.
tests/modules/observability-contract.test.ts(not thetop-level path the plan first named) because
make test-modulesonly gatestests/modules.References
https://lody.ai/leynos/sessions/1b5ce038-a335-4594-a6b9-bbbc09c6e005