fix(collector): stop dropping OTLP Events/Links/Status.Message on ingest (CODE RED #4) + revert OpenApi v3 - #464
Merged
Merged
Conversation
…n ingest (CODE RED #4) OTLP ingest mapped only span identity/timing/status-code/attributes — span Events (including OTel exception events: exception.type/message/stacktrace), Links, and Status.Message were silently dropped, so the span-detail API always returned empty Events/Links and no status message. Full ingest -> storage -> API pipeline (the Qyl.Api.Contracts Span DTO already models these — no contract change): - OtlpConverter.CreateSpanRecordFromProto captures span.Events, span.Links, span.Status.Message. - SpanIngestionRecord + SpanEventIngest/SpanLinkIngest carry them. - IngestionStorageMapper serializes events/links to JSON via SpanChildStorage (source-gen JSON, AOT-safe StorageJsonSerializerContext) and stores status_message. - spans row gains status_message + events_json + links_json columns (and the upsert). - Mappers rehydrates SpanEvent[]/SpanLink[] and SpanStatus.Message from storage instead of []. Also reverts Renovate #462's Microsoft.OpenApi v3 bump back to 2.9.0: v3 makes IOpenApiMediaType.Example read-only, which the SDK's Microsoft.AspNetCore.OpenApi source generator still assigns (CS0200), breaking the whole repo build. 2.9.0 also clears NU1903. Verified: collector Release builds 0/0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Fixes a collector ingest→storage→API gap where OTLP span Events, Links, and Status.Message were not being persisted, ensuring the span DTO returned by the collector can include these fields. Also re-pins Microsoft.OpenApi back to the 2.x line to avoid SDK source-generator breakage introduced by the 3.x upgrade.
Changes:
- Capture OTLP span
Status.Message,Events, andLinksduring ingest, persist them on thespansrow (JSON/VARCHAR columns), and map them back into theSpancontract DTO. - Add AOT-safe source-gen JSON serialization support for the new persisted event/link shapes.
- Revert
Microsoft.OpenApito2.9.0in central package management.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/qyl.collector/Storage/StorageJsonSerializerContext.cs | Adds source-gen JSON metadata for span event/link persisted list shapes. |
| services/qyl.collector/Storage/SpanChildStorage.cs | Introduces persisted JSON shapes + serialize/deserialize helpers for span events/links. |
| services/qyl.collector/Storage/IngestionStorageMapper.cs | Writes status message + events/links JSON onto SpanStorageRow during ingest mapping. |
| services/qyl.collector/Storage/DuckDbReaderExtensions.cs | Extends spans schema/model and upsert conflict update to include new columns. |
| services/qyl.collector/Mapping/Mappers.cs | Rehydrates span events/links/status message from storage into the Span DTO. |
| services/qyl.collector/Ingestion/OtlpIngestionModels.cs | Extends ingest models to carry status message + events/links. |
| services/qyl.collector/Ingestion/OtlpConverter.cs | Extracts events/links/status message from OTLP proto spans. |
| Directory.Packages.props | Pins Microsoft.OpenApi back to 2.9.0 to avoid CS0200 and clear NU1903. |
Comment on lines
+146
to
+153
| foreach (var attr in protoAttributes) | ||
| { | ||
| if (string.IsNullOrEmpty(attr.Key) || !AttributeKeySets.ShouldCaptureSpanAttribute(attr.Key)) | ||
| continue; | ||
|
|
||
| var value = ConvertProtoAnyValue(attr.Value); | ||
| if (value is not null) attributes[attr.Key] = value; | ||
| } |
Comment on lines
+169
to
+178
| private static IReadOnlyList<SpanLinkIngest> BuildSpanLinks(ProtoSpan span) | ||
| { | ||
| if (span.Links.Count is 0) return []; | ||
|
|
||
| var links = new List<SpanLinkIngest>(span.Links.Count); | ||
| foreach (var l in span.Links) | ||
| links.Add(new SpanLinkIngest(ToHex(l.TraceId) ?? "", ToHex(l.SpanId) ?? "", ConvertSpanChildAttributes(l.Attributes))); | ||
|
|
||
| return links; | ||
| } |
ANcpLua
added a commit
that referenced
this pull request
Jul 3, 2026
…deliberately restored VerifyNoRemovedBuildSurface still listed status_message and span.Status?.Message as removed collector surfaces, but #464 reintroduced them on purpose — the gate was defending a dead decision and failing every honest PR touching those files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ANcpLua
added a commit
that referenced
this pull request
Jul 3, 2026
* chore: comment truth sweep — delete fix-markers, dead-pattern defenses, and doc lies - Drop CODE RED #4 fix-markers (Mappers, OtlpConverter, SpanChildStorage, DuckDbReaderExtensions) - Drop historical parentheticals (QylResourceRegistry old channel, ServiceDefaultsSourceGenerator CS9153 wrapper history) - Drop frontend fix-marker + roadmap narration (LogsPage, download-button, Qyl.Run.Host Program) - docs/observability.md: remove false QYL0138/auto-instrumentation composition claim (code only wires ServiceDefaults) - docs/typespec-maf-prd.md: remove 'What stays in qyl' section claiming qyl.conformance lives here (it does not exist in this repo) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * build: stop banning status_message span surfaces the CODE RED #4 fix deliberately restored VerifyNoRemovedBuildSurface still listed status_message and span.Status?.Message as removed collector surfaces, but #464 reintroduced them on purpose — the gate was defending a dead decision and failing every honest PR touching those files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
CODE RED #4 (the last finding). OTLP ingest silently dropped span Events (incl. exception events), Links, and Status.Message. Full ingest→store→map pipeline now persists them (JSON columns on the spans row, AOT-safe source-gen serialization) and rehydrates the
SpanDTO'sEvents/Links/Status.Message— theQyl.Api.ContractsDTO already models these, so no contract change. Also reverts Renovate #462'sMicrosoft.OpenApiv3 bump (breaks the SDK OpenApi source generator repo-wide via CS0200; 2.9.0 also clears NU1903). Verified: collector Release 0/0.🤖 Generated with Claude Code