feat(dev): OTLP trace storage for local dev - #2043
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2043 +/- ##
============================================
+ Coverage 97.10% 97.13% +0.02%
============================================
Files 384 386 +2
Lines 22683 22975 +292
============================================
+ Hits 22027 22316 +289
- Misses 656 659 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
P2, trace filtering: |
| timestamp: new Date(meta.lastSeen).toISOString(), | ||
| sessionId: meta.sessionId, | ||
| spanCount: String(meta.spanCount), | ||
| ...buildTraceDetail(trace.resourceSpans, trace.resourceLogs), |
There was a problem hiding this comment.
every summary carries the full detail, and there's no cap on trace count. and the inspector re-polls this after every invocation. Nothing prunes the dir either. Since ListTracesOptions is new here, a limit for newest-N seems like the cheap fix we should try.
Side note: get() ends up with no frontend caller because of this.
There was a problem hiding this comment.
Added limit. Kept get() — the inspector reads detail through it. Can slim list() to summaries if youd
There was a problem hiding this comment.
ah yes, you're right on get(), useTraces does call the detail endpoint, I was looking at useInvocationTraces mb
There was a problem hiding this comment.
your reply was cut off here btw. what's still open there is that limit slices after every file is read and built + nothing prunes the dir. can we track that as a fast follow?
There was a problem hiding this comment.
Good catch. Tracked as a fast follow: list() builds every file before slicing, and nothing prunes old files. Lands with the Inspector PR.
| * Normalize a trace/span id that may be base64 (protobuf JSON conversion) or | ||
| * already hex (JSON ingest) into lowercase hex. | ||
| */ | ||
| export function hexFromB64OrString(value: string | undefined): string { |
There was a problem hiding this comment.
unrelated, i thought this said B640r and was wondering what type of input it was :(
There was a problem hiding this comment.
i thought the same too. Is it worth explicitly writing out Base64 to make this clearer?
There was a problem hiding this comment.
Renamed hexFromB64OrString to hexFromBase64OrHex in #1980.
| value?: OtlpAttributeValue; | ||
| } | ||
|
|
||
| export type OtlpAttributes = OtlpAttribute[] | Record<string, unknown>; |
There was a problem hiding this comment.
why do we need to accept both formats?
There was a problem hiding this comment.
Fixed to just OtlpAttribute[].
| spanId?: string; | ||
| parentSpanId?: string; | ||
| name?: string; | ||
| kind?: number | string; |
There was a problem hiding this comment.
Fixed, normalizeSpanKind handles both.
0779350 to
d72e3c9
Compare
d81a320 to
613ca5e
Compare
Pure OTLP wire handling (per-trace batch partitioning, id normalization, frontend shaping) and append-only per-trace JSONL storage. A batch routinely carries spans from several traces, so persistence partitions by trace id — writing whole batches under the first id corrupts trace identity. Consumed by the OTLP collector in #1980, which stacks on this.
A distributed trace spans several local agents whose exports append to the same trace file; filtering by any participant must find it, not only the first service seen.
- narrow OtlpAttributes to the key/value wire form (the flat-record variant had no producer); drop the dead passthrough branches - flatten array attributes through extractAnyValue so ints stay numeric and nested kvlists survive (was stringifying and dropping them) - count rendered (post-filter) spans for the list summary instead of raw records, so the count matches the waterfall the inspector shows - surface non-ENOENT fs errors from reads instead of masking them as empty - add newest-N limit to list() for the inspector's per-invocation poll
613ca5e to
4035b7b
Compare
flattenAttributes hand-rolled a value branch per AnyValue kind and had no kvlistValue case, so an attribute whose value is a kvlist (or anything the chain didn't enumerate) silently vanished. Route every attribute value through extractAnyValue, which already unwraps all variants including kvlist — smaller and complete. Addresses Gitika's review on transforms.ts (reuse extractAnyValue; kvlist must not disappear).
| * Normalize a trace/span id that may be base64 (protobuf JSON conversion) or | ||
| * already hex (JSON ingest) into lowercase hex. | ||
| */ | ||
| export function hexFromB64OrString(value: string | undefined): string { |
There was a problem hiding this comment.
i thought the same too. Is it worth explicitly writing out Base64 to make this clearer?
notgitika
left a comment
There was a problem hiding this comment.
I replied to 2 of my earlier comments. they are non-blocking and can be follow ups.
@Hweinstock and I had a nit on a function name for readability if that could also be addressed in the follow up that would be great!
Follow-ups from #2043 review (Gitika, Harrison): - Add a TraceStore.list spanCount test with transport-noise spans (1 agent + 4 http-send -> "1"), guarding the post-filter count. - Rename hexFromB64OrString -> hexFromBase64OrHex; both reviewers misread B64.
Follow-ups from #2043 review (Gitika, Harrison): - Add a TraceStore.list spanCount test with transport-noise spans (1 agent + 4 http-send -> "1"), guarding the post-filter count. - Rename hexFromB64OrString -> hexFromBase64OrHex; both reviewers misread B64.
Follow-ups from #2043 review (Gitika, Harrison): - Add a TraceStore.list spanCount test with transport-noise spans (1 agent + 4 http-send -> "1"), guarding the post-filter count. - Rename hexFromB64OrString -> hexFromBase64OrHex; both reviewers misread B64.
* feat(dev): collect local OTEL traces in project dev An in-process OTLP/HTTP receiver (protobuf via the pinned otlp-transformer decoders, or JSON) persists agent traces through the storage layer. project dev starts it unless --no-traces or the runtime disables instrumentation, points every spawned agent at it (signal-specific OTEL env), rewrites the endpoint to host.docker.internal for containers — with an explicit host-gateway mapping so Linux Docker Engine resolves it — and keeps uvicorn --reload workers instrumented via sitecustomize on PYTHONPATH. Oversized collector requests get a 413 before the connection closes so exporters do not retry them as transient failures. Rebuilt from explicit paths: the previous tree-snapshot commit accidentally reverted unrelated merged work (cdk target guard, config-bundle TUI, error classification). * fix(dev): ack OTLP exports and report trace-persistence failures A batch that can't be persisted (disk full, permissions) was being turned into a 500, which the OTEL SDK exporter retries forever while the user sees nothing. Ack the export (200) so retries stop, and surface the failure once via an onError sink threaded from the collector to the dev handler, which owns the IO to warn the user. Addresses Gitika's review on store.ts:45 (catch in one place; don't let persistence faults read as a silent, retried loss). * test(dev): cover spanCount noise-filtering; clarify hex-id helper name Follow-ups from #2043 review (Gitika, Harrison): - Add a TraceStore.list spanCount test with transport-noise spans (1 agent + 4 http-send -> "1"), guarding the post-filter count. - Rename hexFromB64OrString -> hexFromBase64OrHex; both reviewers misread B64. * docs(dev): clarify collector/httpServer/flag comments per review Harrison review on #1980: - httpServer: generalize the answer-before-close comment (drop OTLP specificity) — the module is a shared io primitive (the Inspector server reuses it). - flags: explain why a default-true boolean is exposed as --no-<name>. - collector: reword the onError comment to state the collector's guarantee (ack + hand to onError) rather than the caller's report-once behavior; drop the volatile "matches the reference CLI" aside. * fix(dev): harden OTEL collector, container reachability, and dev lifecycle Address reviewer findings on the collector and dev wiring: - Validate top-level OTLP shape and return 400 instead of mislabeling a bad payload as a persistence error. - Guard the shared HTTP server against a client that disconnects mid-response so it can no longer crash project dev; add an optional bind host. - Bind the collector to 0.0.0.0 for container runtimes so a container can reach it over the host bridge on Linux. - Run the container template under opentelemetry-instrument so it emits traces. - Keep the collector alive through the child's shutdown grace so final spans are not lost. - Force the OTEL settings that would otherwise let shell or .env.local values disable or break local collection. - Make the uv sitecustomize discovery abortable and read its path from a marker rather than the last merged output line.
First slice of the
project devtracing work; #1980 stacks on this.What this does
The storage half of local trace collection, filesystem I/O only:
otel/transforms.ts— OTLP wire handling: split one export batch into per-trace payloads, normalize protobuf/JSON ids to hex, shape spans/logs for display (attribute flattening, transport-noise filtering). SDKs batch by time, not by trace, so attributing a whole batch to its first trace id would corrupt trace identity — hence the per-trace partitioning.otel/store.ts— append-only storage: one JSON Lines file per trace, read back raw. Malformed lines are skipped so a half-written line can't break a read.Ported from the current CLI's
operations/dev/otel/transforms.ts, with the per-trace batch partitioning as the one correctness fix over the original.Verification
Per-layer tests (happy + unhappy paths), tsc/lint/format clean. End-to-end proof of the full pipeline is in #1980 (real project, real Bedrock invocation, traces on disk).
Replaces #2039, closed by a stacking-tool mishap (base flipped to
main); identical branch and content.