-
Notifications
You must be signed in to change notification settings - Fork 62
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
Updated state interfaces #1325
Updated state interfaces #1325
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
8af10bd
to
e71eb59
Compare
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.
The extractTraceCtx
need to be added back or it'll break trace propagation.
Also added some questions.
if err != nil { | ||
// generate a new one here to be used for subsequent runs. | ||
// this could happen for runs that started before this feature was introduced. | ||
sid := telemetry.NewSpanID(ctx) | ||
fnSpanID = &sid | ||
// TODO: Save span ID // UPDATE? |
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.
it should only save if metadata doesn't have a spanID already stored.
this is likely required if people do future scheduling with event.ts
or just sleeps for a long time.
though the function run will look weird in that case since it'll look like the function started from the middle.
or we just don't care since it's no longer an issue once outside the history window.
e148c5c
to
df95d53
Compare
65cf82a
to
d5c0053
Compare
b1aa107
to
f52e6e9
Compare
cafff08
to
a961136
Compare
We were missing tenant information in some calls to the v2 state interface. We'll be using the accountId for progressively rolling out the new interface.
Embed the event data into pauses so they can be referenced when needed for spans. Also update some data reference due to changes from #1325 CronSchedule is removed from the data structure since it's not referenced anywhere, and is embedded into `Context` map instead. --------- Co-authored-by: Darwin D Wu <darwin67@users.noreply.github.com>
Type of change (choose one)
Checklist
Check our Pull Request Guidelines