feat(cloudflare): instrument scheduled handler #13114
Conversation
AbhiPrasad
requested review from
mydea and
nicohrubec
and removed request for
a team
AbhiPrasad
force-pushed
the
abhi-scheduled-handler
branch
from
20adfb9 to
1482a37
Compare
|
Drafting this PR for the time being because #13123 refactors a lot of this code, so this PR needs to be updated. Hence let's wait for that to get merged in first, and then look at this one in more detail. |
AbhiPrasad
force-pushed
the
abhi-scheduled-handler
branch
3 times, most recently
from
16ddfe8 to
6aa304f
Compare
|
This is now ready to review - I refactored this PR to pull in the refactors from #13123 |
| attributes: { | ||
| 'faas.cron': event.cron, | ||
| 'faas.time': new Date(event.scheduledTime).toISOString(), | ||
| 'faas.trigger': 'timer', |
There was a problem hiding this comment.
These are from https://opentelemetry.io/docs/specs/semconv/faas/faas-spans
|
|
||
| addCloudResourceContext(isolationScope); | ||
|
|
||
| return startSpan( |
There was a problem hiding this comment.
This duplication with the request handler is not ideal, but I'd rather refactor this later once I'm more confident in the API design of the withIsolationScope callback.
| handler.scheduled = new Proxy(handler.scheduled, { | ||
| apply(target, thisArg, args: Parameters<ExportedHandlerScheduledHandler<ExtractEnv<E>>>) { | ||
| const [event, env, context] = args; | ||
| return withIsolationScope(isolationScope => { |
There was a problem hiding this comment.
just for my understanding, what do we need this isolation scope for here?
There was a problem hiding this comment.
To make sure scope bleed doesn't happen. If you define both a scheduled handler and a fetch handler, there's a chance that both happen at the same time, so we need to isolate accordingly.
| captureException(e, { mechanism: { handled: false, type: 'cloudflare' } }); | ||
| throw e; | ||
| } finally { | ||
| context.waitUntil(flush(2000)); |
There was a problem hiding this comment.
why do we need to wait here?
There was a problem hiding this comment.
waitUntil will execute keep the serverless function alive until we flush out all sentry requests, but it will ensure that this happens after a response is sent. Therefore flushing to sentry does not block sending a response back to whatever sent the request to the cloudflare worker.
AbhiPrasad
force-pushed
the
abhi-scheduled-handler
branch
from
a0a3666 to
459be04
Compare
resolves #13112
This PR adds instrumentation for the
scheduledhandler in cloudflare workers. This is used for cron triggers.I elected to not do automatic cron instrumentation for now, this is tracked by #13113. Instead I added manual instrumentation docs to the README, this will get copied to the sentry docs eventually.
ref #12620