Skip to content

feat(deno): add Deno.serve integration #12460

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

brc-dd
Copy link

@brc-dd brc-dd commented Jun 11, 2024

closes #12450

the code is mostly taken from the bun sdk

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@AbhiPrasad AbhiPrasad requested review from timfish and AbhiPrasad June 11, 2024 15:48
Copy link
Collaborator

@timfish timfish left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -488,7 +488,7 @@ jobs:
- name: Set up Deno
uses: denoland/[email protected]
with:
deno-version: v1.38.5
deno-version: v1.x
Copy link
Author

@brc-dd brc-dd Jun 12, 2024

Choose a reason for hiding this comment

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

jsr wasn't supported on deno 1.38. deno.land/std/... won't receive non-critical updates and jsr is the recommended way

if needed we can lock it to some minor

re-running CI should pass I guess 👀

return new Proxy(handler, {
apply(handlerTarget, handlerThisArg, handlerArgs: Parameters<RawHandler>) {
return withIsolationScope(isolationScope => {
isolationScope.clear();
Copy link
Author

Choose a reason for hiding this comment

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

also this line should be removed when #12450 (comment) is resolved

@mydea
Copy link
Member

mydea commented May 20, 2025

sorry, this has been open for a very long time - is this still relevant to the state of deno today (where we are basically node-compatible)? This has run quite out of sync and would need rebasing on develop.

@brc-dd
Copy link
Author

brc-dd commented May 20, 2025

is this still relevant to the state of deno today

Yes. Deno.serve is still a separate API. Node code won't instrument it. This PR is blocked by #15455 (see #12450 (comment))

where we are basically node-compatible

Node.js compatibility here means you will be able to use Sentry's Node.js httpIntegration but that will only apply to Node.js-style servers (the ones directly or indirectly using node:http in Deno).

Also, even then the Node.js compatibility mode and Deno globals are different (see #12400 and denoland/deno#20826) and since Sentry v9 is no longer published to deno.land and the JSR version is pretty much unusable, there is no official way for stuff like console integration to work with Deno.

This has run quite out of sync and would need rebasing on develop.

Yeah, I can rebase it, but I don't think it's gonna be unblocked anytime soon, so it will get stale again 🤷‍♂️


However, if the OLTP endpoint lands, then it will fix both these issues. See #85902 and denoland/deno#28330 (comment)

On a side note, we have been using a patched version in prod for almost an year now - https://github.com/globalbrain/hado/blob/main/src/sentry.ts - so if anyone using Deno wants to use Sentry with fixes for console and fetch capturing and Deno.serve integration, they can use it.

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.

Support Deno.serve instrumentation
3 participants