-
Notifications
You must be signed in to change notification settings - Fork 61
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
Support step.waitForEvent
in trace runs
#1358
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
step.waitForEvent
step.waitForEvent
in trace runs
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.
This was the span reconstruction that wasn't seeming to work last week, though was probably just something silly. Worth checking they're coming through correctly.
span.Send() | ||
defer span.End() | ||
span.SetAttributes(commonAttrs...) |
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.
Could we be intensely unlucky here and export the span before the second call to SetAttributes()
?
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.
addressed in #1360
@@ -2200,6 +2227,7 @@ func (e *executor) handleGeneratorInvokeFunction(ctx context.Context, gen state. | |||
), | |||
) | |||
span.Send() | |||
defer span.End() |
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.
Any reason to do this? span.Send()
calls span.End()
anyway and we don't add any details to the span later on.
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.
addressed in #1360
telemetry.WithName(consts.OtelSpanWaitForEvent), | ||
telemetry.WithTimestamp(now), | ||
telemetry.WithSpanAttributes( | ||
attribute.Bool(consts.OtelUserTraceFilterKey, true), |
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.
This will need some attributes adding to fulfil the resolvers, namely:
- Event name
- Expression
- Expiry time
telemetry.WithSpanAttributes( | ||
attribute.String(consts.OtelSysStepOpcode, enums.OpcodeWaitForEvent.String()), | ||
attribute.Int64(consts.OtelSysStepWaitExpires, pause.Expires.Time().UnixMilli()), | ||
attribute.Bool(consts.OtelSysStepWaitExpired, r.EventID == nil), | ||
), |
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.
We'll also need to add some more info for the resolvers, namely:
- Event name
- Expression
- ID of the found event (if any)
Description
Add span for showing the step is waiting for an event.
Type of change (choose one)
Checklist
Check our Pull Request Guidelines