Repository navigation
Allow product-specific step span attributes #3825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@workflow/web-shared': patch | ||
| --- | ||
|
|
||
| Allow trace callers to add product-specific attributes to event-derived step spans. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,10 @@ export function waitToSpan( | |
| }; | ||
| } | ||
|
|
||
| export type GetStepAttributes = ( | ||
| events: Event[] | ||
| ) => Record<string, unknown> | undefined; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AI Review: The return type suggests any product-specific key can be surfaced, but the sidebar drops keys that are not registered in |
||
|
|
||
| export const stepEventsToStepEntity = ( | ||
| events: Event[] | ||
| ): { | ||
|
|
@@ -231,7 +235,11 @@ export const stepEventsToStepEntity = ( | |
| /** | ||
| * Converts step events to an OpenTelemetry Span | ||
| */ | ||
| export function stepToSpan(stepEvents: Event[], maxEndTime: Date): Span | null { | ||
| export function stepToSpan( | ||
| stepEvents: Event[], | ||
| maxEndTime: Date, | ||
| getStepAttributes?: GetStepAttributes | ||
| ): Span | null { | ||
| const step = stepEventsToStepEntity(stepEvents); | ||
| if (!step) { | ||
| return null; | ||
|
|
@@ -242,7 +250,11 @@ export function stepToSpan(stepEvents: Event[], maxEndTime: Date): Span | null { | |
|
|
||
| const attributes = { | ||
| resource: 'step' as const, | ||
| data: step, | ||
| data: { | ||
| ...getStepAttributes?.(stepEvents), | ||
| // Canonical event-derived fields cannot be overridden by extensions. | ||
| ...step, | ||
| }, | ||
| }; | ||
|
|
||
| const resource = 'step'; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,11 @@ let nextId = 0; | |
|
|
||
| function event( | ||
| eventType: EventType, | ||
| options: { correlationId?: string; at: number } | ||
| options: { | ||
| correlationId?: string; | ||
| at: number; | ||
| externalAttemptId?: string; | ||
| } | ||
| ): Event { | ||
| nextId += 1; | ||
| return { | ||
|
|
@@ -20,6 +24,7 @@ function event( | |
| createdAt: new Date(BASE_TIME + options.at * 1000), | ||
| occurredAt: new Date(BASE_TIME + options.at * 1000), | ||
| eventData: eventType === 'step_created' ? { stepName: 'doWork' } : {}, | ||
| externalAttemptId: options.externalAttemptId, | ||
| } as unknown as Event; | ||
| } | ||
|
|
||
|
|
@@ -31,6 +36,42 @@ const run = { | |
| } as unknown as WorkflowRun; | ||
|
|
||
| describe('buildTrace', () => { | ||
| it('adds caller-derived attributes to step span data', () => { | ||
| const events = [ | ||
| event('run_created', { at: 0 }), | ||
| event('run_started', { at: 0 }), | ||
| event('step_created', { correlationId: 'step_a', at: 1 }), | ||
| event('step_started', { | ||
| correlationId: 'step_a', | ||
| at: 2, | ||
| externalAttemptId: 'attempt_first', | ||
| }), | ||
| event('step_retrying', { correlationId: 'step_a', at: 3 }), | ||
| event('step_started', { | ||
| correlationId: 'step_a', | ||
| at: 4, | ||
| externalAttemptId: 'attempt_latest', | ||
| }), | ||
| ]; | ||
|
|
||
| const trace = buildTrace(run, events, new Date(BASE_TIME + 5000), { | ||
| getStepAttributes(stepEvents) { | ||
| const latestStart = stepEvents | ||
| .slice() | ||
| .reverse() | ||
| .find((candidate) => candidate.eventType === 'step_started') as | ||
| | (Event & { externalAttemptId?: string }) | ||
| | undefined; | ||
| return { externalAttemptId: latestStart?.externalAttemptId }; | ||
| }, | ||
| }); | ||
| const stepSpan = trace.spans.find((span) => span.resource === 'step'); | ||
|
|
||
| expect(stepSpan?.attributes.data).toMatchObject({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| externalAttemptId: 'attempt_latest', | ||
| }); | ||
| }); | ||
|
|
||
| it('ends a step span on the terminal event the run acted on', () => { | ||
| const events = [ | ||
| event('run_created', { at: 0 }), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 accessexternalAttemptId, and Front will need the same workaround forvercelId/computeInstanceId. Could we makeGetStepAttributes,buildTrace, andTraceViewergeneric overTEvent extends Eventso consumers retain their enriched event type?