Allow product-specific step span attributes - #3825
Conversation
🦋 Changeset detectedLatest commit: b8647a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🧪 E2E Test Results⏳ Tests are running... This comment will be updated with the results when the tests complete. _Started at: _ |
📊 Workflow Benchmarks⏳ Benchmarks are running for ℹ️ Metric definitions & methodologyMetrics — All timestamps are deployment-side; runs are triggered in-deployment, so the CI runner and api.vercel.com sit outside every measured window. TTFS = Cold starts stay in the numbers (real bursty-workload latency, inflates P75+); Best is the warm floor. |
7b53fac to
3d1fb2c
Compare
Sim WorldSimulated world deterministic testing for races. Traces 🟠 world-sim scenario book — 1 fail of 41 total
Full trace: |
About these numbersSizes are gzip; parentheses show the change against
|
3d1fb2c to
ce72f03
Compare
ce72f03 to
1e2d97c
Compare
515fc7c to
41a11bc
Compare
Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
41a11bc to
b8647a4
Compare
karthikscale3
left a comment
There was a problem hiding this comment.
AI Review: Approved with a few non-blocking API and test follow-ups.
| } | ||
|
|
||
| export type GetStepAttributes = ( | ||
| events: Event[] |
There was a problem hiding this comment.
AI Review: This callback is meant to read product-specific enrichment fields, but its input is fixed to the base Event[]. The new test already has to cast to access externalAttemptId, and Front will need the same workaround for vercelId / computeInstanceId. Could we make GetStepAttributes, buildTrace, and TraceViewer generic over TEvent extends Event so consumers retain their enriched event type?
|
|
||
| export type GetStepAttributes = ( | ||
| events: Event[] | ||
| ) => Record<string, unknown> | undefined; |
There was a problem hiding this comment.
AI Review: The return type suggests any product-specific key can be surfaced, but the sidebar drops keys that are not registered in attributeToDisplayFn. The current Vercel fields happen to be registered, so this works for this use case; please either document that display registration is also required or expose a way for consumers to provide display metadata.
| }); | ||
| const stepSpan = trace.spans.find((span) => span.resource === 'step'); | ||
|
|
||
| expect(stepSpan?.attributes.data).toMatchObject({ |
There was a problem hiding this comment.
AI Review: Could we add a collision assertion here? The implementation intentionally spreads canonical step fields after extension attributes, so a test where the callback returns stepId or status would lock in the non-overridable guarantee documented in the implementation.
karthikscale3
left a comment
There was a problem hiding this comment.
would be useful to add a link to the logs page from the side bar and filtering it by the requestId.
|
No backport to This commit adds a new optional To override, re-run the Backport to stable workflow manually via |
Summary & Motivation
Products such as the Vercel dashboard enrich canonical workflow events with backend-specific metadata.
buildTracenow takes an optionalgetStepAttributescallback so those attributes reach step span details without@workflow/web-sharedknowing the fields; canonical event-derived fields win on key collision.Test Plan
Unit test added.