Skip to content

feat(cloudflare): Add instrumentWorkflowWithSentry to instrument workflows #16672

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

Merged
merged 9 commits into from
Jun 23, 2025

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Jun 22, 2025

It was tricky to instrument Cloudflare workflows! The code for a workflow might look simple but there is a lot of clever shenanigans going on under the hood in the runtime to allow suspension of workflows. Workflows can be hibernated at any time and all state/context inside the your workflow class and elsewhere is lost. Ideally we want all of our step runs to have the same trace_id so all steps in a workflow run are linked together and all steps should have the same sampling decision.

To work around the state limitations, we use the workflow instanceId as both the Sentry trace_id and the last 4 characters are used to generate the sample_rand used in the sampling decision. Cloudflare uses uuid's by default for instanceId but users do have the option of passing their own IDs. If users are supplying their own instanceId's, they need to be both random and a 32 character uuid (with or without hyphens) or the Sentry instrumentation will throw an error.

Points worthy of note:

  • We use a enableDedupe config option (docs hidden) which removes the dedupeIntegration for workflows. We want to get duplicate errors for step retries
  • We have to wrap the Cloudflare WorkflowStep object in another class. The Cloudflare step object is native so it's properties can't be overridden or proxied
    • Our wrapping does end up in all the stack traces but should be automatically hidden because they will be evaluated as in_app: false
  • We don't wrap step.sleep, step.sleepUntil or step.waitForEvent because code doesn't run after the Cloudflare native function returns ☹️
  • Calling setPropagationContext directly on the isolation context didn't work. It needed another withScope inside for setPropagationContext to work. @mydea is that expected?
  • This PR doesn't yet capture:
    • The payload supplied when the workflow run was started
    • The return results from the workflow steps

Here is an example trace showing the final step failing (throwing) 6 times before completing successfully. The exponential retry backoff is clearly visible.

image

return wrapRequestHandler({ options, request: context.request, context }, () => context.next());
return wrapRequestHandler({ options, request: context.request, context: { ...context, props: {} } }, () =>
context.next(),
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was due to updating to the latest Cloudflare types.

ExecutionContext now has a props property which isn't on EventPluginContext.

@@ -34,7 +34,7 @@ export function initCloudflareSentryHandle(options: CloudflareOptions): Handle {
return wrapRequestHandler(
{
options: opts,
request: event.request,
request: event.request as Request<unknown, IncomingRequestCfProperties<unknown>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type updates also broke this...

Copy link

codecov bot commented Jun 22, 2025

❌ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:

Error parsing JUnit XML in /home/runner/work/sentry-javascript/sentry-javascript/packages/solidstart/vitest.junit.xml at 223:249

Caused by:
    RuntimeError: Error parsing XML
    
    Caused by:
        0: ill-formed document: expected `</system-out>`, but `</system-out          </testcase        <testcase classname="test/config/withSentry.test.ts" name="withSentry() &gt; does not add the instrumentation file as top level import if autoInjectServerSentry is undefined" time="0.005488011">` was found
        1: expected `</system-out>`, but `</system-out          </testcase        <testcase classname="test/config/withSentry.test.ts" name="withSentry() &gt; does not add the instrumentation file as top level import if autoInjectServerSentry is undefined" time="0.005488011">` was found

For more help, visit our troubleshooting guide.

@timfish timfish marked this pull request as ready for review June 22, 2025 16:42
@timfish timfish requested a review from AbhiPrasad June 22, 2025 16:42
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work - just some small comments.

timfish and others added 5 commits June 23, 2025 15:43
@timfish timfish requested a review from AbhiPrasad June 23, 2025 15:34
@AbhiPrasad AbhiPrasad merged commit 6858602 into develop Jun 23, 2025
318 of 320 checks passed
@AbhiPrasad AbhiPrasad deleted the timfish/feat/cloudflare-workflows branch June 23, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Cloudflare Workflows
2 participants