[OPIK-7717] [QA] Proposed e2e specs for the OTel semconv provider aliases (from the #7909 exploration) - #7945
Conversation
Two proposed specs covering the provider-alias behaviour from #7909, written from a human-verified exploration of that PR's own environment. - traces.span-model-cost-tokens: an OTLP-ingested Claude-on-Vertex span resolves to anthropic_vertexai and prices from the Anthropic rows ($0.0035), while an identically-tokened Gemini span prices from the Vertex language-model rows ($0.0003). Asserted via GET /v1/private/spans and on the span-detail chip. - traces.toggle-spans-view (was covered: false): the Spans tab's provider filter matches OTLP spans under their canonical provider name, 1 vs 2 rows across the two providers. Supporting: an otelSpans fixture seeding through the OTLP endpoint, ingestOtelSpans/listSpans/waitForSpansByName on the backend client, Spans-tab support on LogsPage, and waitForSpanSelected on TracePanelPage. LogsPage.applyFilter now blurs the value input before closing the chip: the value cells wrap a DebounceInput whose pending callback is cancelled on unmount, so closing within the 300ms window applied no filter at all. The existing callers only avoided this because the autocomplete cells blur themselves on Escape; the plain-text provider chip does not. Generated by the release QA side flow — draft, needs review before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
tests
📋 PR Linter Failed❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the |
⏱️ pre-commit per-hook timingNo linted files changed — nothing to run. ⏭️ 43 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| id: String(s.id ?? ''), | ||
| traceId: String(s.traceId ?? ''), |
There was a problem hiding this comment.
Missing span IDs reach navigation
The mapper converts omitted SpanPublic.id and traceId into '' in SpanRowRef, while waitForSpansByName accepts the row by name alone and copies those values into OtelSpanRef, so openSpanById builds selectors/URLs from empty values and the detail assertion targets no row instead of rejecting the malformed response. Although the current spans table normally stores non-null FixedString IDs, should we require non-empty id and traceId during mapping—or in the waiter’s success predicate—since the published Span_Public schema does not require them?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
tests_end_to_end/e2e/core/backend/client.ts around lines 314-315, update the
`localListSpans` mapper so optional SDK `id` and `traceId` fields are not coerced to
empty strings in `SpanRowRef`. Validate that both values are non-empty before returning
each span, and throw a descriptive error for malformed backend responses (optionally
also require both fields in the `waitForSpansByName` success predicate). Add or update
coverage to ensure spans with missing identity fields are rejected at the client
boundary.
| const toAttribute = (key: string, value: string | number) => ({ | ||
| key, | ||
| // OTLP/JSON encodes 64-bit ints as strings; the token-usage attributes | ||
| // are the ones that must arrive as ints or the price lookup sees none. | ||
| value: | ||
| typeof value === 'number' ? { intValue: String(value) } : { stringValue: value }, | ||
| }); |
There was a problem hiding this comment.
Non-integer attributes reject whole batches
The public OtelSpanSeed.attributes contract accepts any JavaScript number, but this branch encodes every value as protobuf int64 intValue, so 1.5, NaN, and Infinity are rejected by the backend's JsonFormat decoder and the entire OTLP request fails before any span is stored. Could we restrict or validate integer attributes with Number.isFinite(value) && Number.isInteger(value), or emit finite non-integers as doubleValue?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
tests_end_to_end/e2e/core/backend/client.ts around lines 623-629, update the
`ingestOtelSpans` `toAttribute` helper so its handling matches the public
`OtelSpanSeed.attributes` contract. Reject non-finite numeric values with a clear error
before building the request, encode integers as OTLP `intValue`, and encode finite
non-integers as `doubleValue`; preserve string attributes as `stringValue` and update
nearby comments/types if needed.
| spans: args.spans.map((span) => ({ | ||
| traceId: span.traceId, | ||
| spanId: span.spanId, |
There was a problem hiding this comment.
OTLP IDs are remapped to wrong identities
OtelSpanSeed sends 32/16-character hex traceId/spanId values straight to the protobuf JsonFormat reader, which treats them as base64 and decodes them to 24/12 bytes, and OpenTelemetryService still returns 200 for these — so persisted Opik IDs no longer match the supplied OTLP identities and !res.ok can't catch it. Should we hex-decode the IDs into 16/8 raw bytes and base64-encode those for the payload, validating lengths/characters first?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
tests_end_to_end/e2e/core/backend/client.ts around lines 640-642, fix `ingestOtelSpans`
so the OTLP `traceId` and `spanId` values are not sent as raw hex strings to the
protobuf JSON reader, which interprets them as base64 and produces incorrectly sized
IDs. Validate that each ID is hexadecimal with exactly 32 characters for `traceId` and
16 for `spanId`, decode the hex into 16/8 raw bytes, and base64-encode those bytes for
the request payload; throw a clear error for invalid lengths or characters.
| if (!res.ok) { | ||
| throw new Error( | ||
| `POST /v1/private/otel/v1/traces -> ${res.status}: ${(await res.text()).slice(0, 300)}`, | ||
| ); |
There was a problem hiding this comment.
Unsanitized OTLP errors leak into CI output
The thrown Playwright error interpolates the first 300 characters of the remote response verbatim, so parser-failure ErrorMessage bodies built from exception.getMessage() (including caller payload details) can inject newlines or expose sensitive content in CI artifacts when OPIK_BASE_URL targets cloud or self-hosted deployments. Should we retain the 300-character bound but sanitize to a single-line safe excerpt and mask approved secret/token patterns before interpolation?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`tests_end_to_end/e2e/core/backend/client.ts` around lines 674-677, update
`ingestOtelSpans` so failed OTLP requests do not interpolate the remote response body
verbatim into the Playwright error. Read the body once, create a single-line safe
excerpt limited to 300 characters, replace or escape control characters, and mask
approved secret/token patterns before including it in the error message. Preserve the
status and existing error behavior.
| while (Date.now() - start < timeoutMs) { | ||
| const { spans } = await listSpans({ projectId }); | ||
| seen = spans.map((s) => s.name); |
There was a problem hiding this comment.
Transient ingestion failure aborts polling
A transient listSpans failure makes waitForSpansByName reject immediately, so asynchronous OTLP ingestion cannot recover within the polling window and surfaces as a startup/readiness failure — should we retain intermediate errors, continue polling until timeoutMs, and include the last error's message/String(error) in the final timeout error?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`tests_end_to_end/e2e/core/backend/wait-for-spans.ts` around lines 35-37, update
`waitForSpansByName` so a transient `listSpans` error does not reject the function
immediately. Catch each polling error, retain the most recent error while continuing to
poll until `timeoutMs`, and include `String(error)` or its message in the final timeout
error alongside the missing and present spans.
1 participant
What this is
Two proposed e2e specs covering the OTel provider-alias behaviour shipped by
#7909 (
[OPIK-7717] [BE] fix: map OTel semconv provider names to Opik canonical providers).They came out of an exploratory pass over #7909 on its own deployed environment
(
pr-7909.dev.comet.com,2.2.36-7909-merge-3030). A human drove each flow byhand there first and confirmed it works; only verified flows were turned into
specs.
Base branch:
OPIK-7717-otel-provider-aliases, notmain. These specsassert behaviour that exists only in #7909 —
anthropic_vertexaion aClaude-on-Vertex span, and a non-zero cost for it — so they would fail on
mainuntil that PR merges. They were written and run against its head commit
e377b41. When #7909 merges, this PR retargets tomaincleanly.The specs
Both live in
tests_end_to_end/e2e/tests/trace-explore/otel-span-provider.spec.ts,tagged
@t2-cuj/@area:traces.1.
An OTLP-ingested Claude-on-Vertex span resolves to anthropic_vertexai and is priced from the Anthropic rows@cap:traces.span-model-cost-tokensPushes two spans through
POST /v1/private/otel/v1/traceswith identical tokencounts (1000 in / 500 out) —
gen_ai.system=vertex_aiwithclaude-haiku-4-5,and the same provider with
gemini-2.5-flash-lite— then asserts:GET /v1/private/spans: provideranthropic_vertexaiat $0.0035 forthe Claude span and
google_vertexaiat $0.0003 for the Gemini one, overthe project's whole span set (count included), not just the rows it looked up;
anthropic_vertexai claude-haiku-4-5and the cost renders
<$0.01.The exact costs are the point. Same tokens on both spans means only the resolved
provider can move the number, and the pre-fix behaviour — a Claude model left
under
google_vertexaimatching no price row — renders a perfectly ordinary spanthat costs
$0.trace-spans-depth.spec.tsalready covers this capability, butonly through the Python SDK with a provider the test itself chose, so it cannot
fail on the OTel mapping.
Verification: passed.
2.
The Spans tab provider filter matches OTLP spans under their canonical provider name@cap:traces.toggle-spans-view— previouslycovered: falsein the taxonomy("Logs has a 3rd toggle (Spans); only Traces+Threads covered").
Seeds three OTLP spans (Claude via
gen_ai.system, Gemini viagen_ai.system,Gemini via the semconv-current
gen_ai.provider.name=gcp.vertex_ai), confirmsvia the API that they really split 1/2 across the two providers, then clicks the
Spans toggle and filters:
provider contains anthropic_vertexai→ exactly the 1 Claude span;provider contains google_vertexai→ exactly the other 2.Each step asserts the row count and which rows survived, so a filter that
returned the right number of wrong rows fails. The
gcp.vertex_aispan is thediscriminator for the attribute half: a chain still reading
gen_ai.systemalone would leave it with no provider at all.
Verification: passed.
Supporting changes
fixtures/otel-spans.fixture.ts— new fixture, chained and re-exported likethe others. Seeding lives here rather than in a trailing cleanup step, and
teardown rides on the
projectfixture's cascade (same asfilterable-traces.fixture.ts). It polls for the seeded span names after thePOST — OTLP ingestion answers 200 before the spans are queryable — and throws
naming what never landed rather than handing the spec a partial set.
core/backend/client.ts—ingestOtelSpans(raw fetch: OTLP has no SDKsurface, and it is the only path the alias chain runs on),
listSpans, andwaitForSpansByNameincore/backend/wait-for-spans.ts.pom/logs.page.ts— Spans-tab support: the toggle, span rows keyed bydata-row-id,spanCellviadata-cell-id, andopenSpanById.pom/trace-panel.page.ts—waitForSpanSelected, because a panel openedstraight at
?trace=…&span=…mounts on the trace and swaps to the span a beatlater.
coverage/taxonomy.yaml— spec added to the area'sspecs:list;toggle-spans-viewflipped tocovered: true, tier: t2-cuj.One shared-POM fix, worth a look
LogsPage.applyFilternow blurs the value input before closing the popover.Every filter value cell wraps a
DebounceInputwhose pending callback iscancelled on unmount, so closing the chip within the 300ms debounce window
discards the typed value and applies no filter at all. The existing callers hit
this only by luck: they drive autocomplete cells, whose own Escape handler blurs
the input and flushes the debounce first. A plain text cell — which is what the
providerchip is — has no such handler, and the filter silently did nothing.Blur is the FE's designed flush path (
DebounceInput.handleBlurcalls.flush()), and it is the same idiomTracePanelPage.setAnnotateScorealreadyuses.
The whole
tests/trace-explore/directory was re-run after this change: 19passed, 1 flaky —
trace-filters.spec.ts › Feedback score filter …, whichtimed out opening its chip popover and passed on retry.
That one is pre-existing and unrelated: it goes through
pinFilterChip+applyKeyedFilter, neither of which this PR touches, and it fails the same waywith the
applyFilterchange reverted (run in isolation with--repeat-each=4,it failed every attempt on both sides of the change). Flagging it because
applyKeyedFilterlooks like it has the same un-flushed-debounce problemapplyFilterhad — worth a separate fix, not folded in here.What I deliberately did not write
One candidate from the exploration was dropped: "a fully-migrated
execute_toolspan carryinggen_ai.provider.nameis not retyped as an LLMcall." The exploration ranked it
weakon the grounds that it overlaps thefirst spec's seeding and reads more naturally as one more assertion there than
as a spec of its own. It verified fine by hand (
type=tool, no cost, Logsrenders Tool) and it does guard a real regression this PR's review feedback
caught, so it is a reasonable follow-up — it is left out because the ranking said
weak and there were stronger candidates, not because it failed.
Nothing was dropped for failing to run.
How this was verified
Target: an OSS install of the #7909 build (
OPIK_DEPLOYMENT=oss, workspacedefault).tscclean; tag-lint0 problem(s).🤖 Generated by the release QA side flow (
release-test-proposal) from ahuman-verified exploration of #7909. Draft on purpose — it needs a human
review before merge. The assertions are only as good as the exploration they
came from; please check that each
@cap:tag matches what the test actuallyasserts.