Skip to content

feat(bun): Instrument Bun.serve #9080

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 3 commits into from
Sep 21, 2023
Merged

Conversation

AbhiPrasad
Copy link
Member

ref #9042

Add automatic instrumentation for Bun.serve

Simply do something like so:

import * as Sentry from '@sentry/bun';

Sentry.init({
  dsn: Bun.env.SENTRY_DSN,
  tracesSampleRate: 1.0,
});

// later

Bun.serve({
  async fetch() {
    await new Promise((resolve) => setTimeout(resolve, 1000));
    throw new Error("An Error from Bun!");
    return new Response("Bun!");
  },
});

and then get:

image

image

Link: https://sentry-sdks.sentry.io/discover/abhi-sentry-bun:f2e553067c7f49d58624963ca5e544ae/?field=title&field=event.type&field=project&field=user.display&field=timestamp&field=replayId&homepage=true&name=All+Events&project=4505841905238016&query=&sort=-timestamp&statsPeriod=1h&yAxis=count%28%29

Follow up note. We probably want to export a set of utilities for dealing with fetch + plain Response and Request web APIs for Sentry. For now decided to not do that so we can ship this faster.

@AbhiPrasad AbhiPrasad requested review from HazAT, a team and stephanie-anderson and removed request for a team September 21, 2023 01:28
@AbhiPrasad
Copy link
Member Author

Bun tests are so crazy fast...

~/workspace/sentry-javascript/packages/bun (abhi-bun-server-instrumentation) » bun test                                abhijeetprasad@GT9RQ02WW5
bun test v1.0.0 (822a00c4)

test/sdk.test.ts:
✓ calling init shouldn't fail [2.98ms]

test/integrations/bunserver.test.ts:
✓ Bun Serve Integration > generates a transaction around a request [5.54ms]
✓ Bun Serve Integration > generates a post transaction [1.69ms]
✓ Bun Serve Integration > continues a trace [2.99ms]
✓ Bun Serve Integration > does not create transactions for OPTIONS or HEAD requests [1.46ms]

 5 pass
 0 fail
 13 expect() calls
Ran 5 tests across 2 files. [135.00ms]

@lforst lforst self-requested a review September 21, 2023 06:24
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

clean <3

* Instruments Bun.serve by patching it's options.
*/
export function instrumentBunServe(): void {
Bun.serve = new Proxy(Bun.serve, {
Copy link
Member

Choose a reason for hiding this comment

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

dayum we're lucky this is just a field on a global - let's pray this never changes 🙏

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think they'll break this for a while, but let's see.


const request = fetchArgs[0];
const upperCaseMethod = request.method.toUpperCase();
if (!options || upperCaseMethod === 'OPTIONS' || upperCaseMethod === 'HEAD') {
Copy link
Member

Choose a reason for hiding this comment

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

m: I might be blind but why do we check for the non-existence of options here? Is there anything preventing us from early-returning all the way at the top on requests with OPTIONS and HEAD?

Copy link
Member Author

Choose a reason for hiding this comment

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

will adjust.

Comment on lines 12 to 14
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
// store a seen flag on it.
const objectifiedErr = objectify(e);
Copy link
Member

Choose a reason for hiding this comment

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

m: Doesn't captureException already do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied what we were doing in SvelteKit, but I think our event processing pipeline does do this. Let me adjust.

import { instrumentBunServe } from '../../src/integrations/bunserver';
import { getDefaultBunClientOptions } from '../helpers';

// Fun fact: Bun = 2 21 14 :)
Copy link
Member

Choose a reason for hiding this comment

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

:)

type: 'bun',
handled: false,
data: {
function: 'serve',
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't sure if serve or fetch is more correct but w/e

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah debated this too, will stick with serve for now, but we can always adjust later.

Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

pretty sweet! 🚀

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) September 21, 2023 15:42
@AbhiPrasad AbhiPrasad merged commit 8001498 into develop Sep 21, 2023
@AbhiPrasad AbhiPrasad deleted the abhi-bun-server-instrumentation branch September 21, 2023 15:44
@AbhiPrasad AbhiPrasad self-assigned this Sep 21, 2023
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.

3 participants