Skip to content

fix(bun): Ensure instrumentation of Bun.serve survives a server reload #15148

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,18 @@ export function instrumentBunServe(): void {
Bun.serve = new Proxy(Bun.serve, {
apply(serveTarget, serveThisArg, serveArgs: Parameters<typeof Bun.serve>) {
instrumentBunServeOptions(serveArgs[0]);
return serveTarget.apply(serveThisArg, serveArgs);
const server: ReturnType<typeof Bun.serve> = serveTarget.apply(serveThisArg, serveArgs);

// A Bun server can be reloaded, re-wrap any fetch function passed to it
// We can't use a Proxy for this as Bun does `instanceof` checks internally that fail if we
// wrap the Server instance.
const originalReload: typeof server.reload = server.reload.bind(server);
server.reload = (serveOptions: Parameters<typeof Bun.serve>[0]) => {
instrumentBunServeOptions(serveOptions);
return originalReload(serveOptions);
};

return server;
},
});
}
Expand Down
130 changes: 94 additions & 36 deletions packages/bun/test/integrations/bunserver.test.ts
Original file line number Diff line number Diff line change
@@ -1,67 +1,87 @@
import { beforeAll, beforeEach, describe, expect, test } from 'bun:test';
import { afterEach, beforeAll, beforeEach, describe, expect, test } from 'bun:test';
import type { Span } from '@sentry/core';
import { getDynamicSamplingContextFromSpan, setCurrentClient, spanIsSampled, spanToJSON } from '@sentry/core';

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

// Fun fact: Bun = 2 21 14 :)
const DEFAULT_PORT = 22114;

describe('Bun Serve Integration', () => {
let client: BunClient;
Copy link
Contributor Author

@nathankleyn nathankleyn Jan 23, 2025

Choose a reason for hiding this comment

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

I discovered that none of the tests in this file were actually working — all of the client.on('spanEnd', ...) were never being called and since all the expects were inside of them nothing failed.

I have restructured the tests to make sure that if the events ever stop firing again in the future, the tests will fail, then fixed all the tests.

// Fun fact: Bun = 2 21 14 :)
let port: number = 22114;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't reuse the port on each start since server.stop happens in the background and tests end up using each other's servers.

beforeAll(() => {
instrumentBunServe();
});

beforeEach(() => {
const options = getDefaultBunClientOptions({ tracesSampleRate: 1, debug: true });
const options = getDefaultBunClientOptions({ tracesSampleRate: 1 });
client = new BunClient(options);
setCurrentClient(client);
client.init();
});

afterEach(() => {
// Don't reuse the port; Bun server stops lazily so tests may accidentally hit a server still closing from a
// previous test
port += 1;
});

test('generates a transaction around a request', async () => {
let generatedSpan: Span | undefined;

client.on('spanEnd', span => {
expect(spanToJSON(span).status).toBe('ok');
expect(spanToJSON(span).data?.['http.response.status_code']).toEqual(200);
expect(spanToJSON(span).op).toEqual('http.server');
expect(spanToJSON(span).description).toEqual('GET /');
generatedSpan = span;
});

const server = Bun.serve({
async fetch(_req) {
return new Response('Bun!');
},
port: DEFAULT_PORT,
port,
});
await fetch(`http://localhost:${port}/`);
server.stop();

await fetch('http://localhost:22114/');
if (!generatedSpan) {
throw 'No span was generated in the test';
}

server.stop();
expect(spanToJSON(generatedSpan).status).toBe('ok');
expect(spanToJSON(generatedSpan).data?.['http.response.status_code']).toEqual(200);
expect(spanToJSON(generatedSpan).op).toEqual('http.server');
expect(spanToJSON(generatedSpan).description).toEqual('GET /');
});

test('generates a post transaction', async () => {
let generatedSpan: Span | undefined;

client.on('spanEnd', span => {
expect(spanToJSON(span).status).toBe('ok');
expect(spanToJSON(span).data?.['http.response.status_code']).toEqual(200);
expect(spanToJSON(span).op).toEqual('http.server');
expect(spanToJSON(span).description).toEqual('POST /');
generatedSpan = span;
});

const server = Bun.serve({
async fetch(_req) {
return new Response('Bun!');
},
port: DEFAULT_PORT,
port,
});

await fetch('http://localhost:22114/', {
await fetch(`http://localhost:${port}/`, {
method: 'POST',
});

server.stop();

if (!generatedSpan) {
throw 'No span was generated in the test';
}

expect(spanToJSON(generatedSpan).status).toBe('ok');
expect(spanToJSON(generatedSpan).data?.['http.response.status_code']).toEqual(200);
expect(spanToJSON(generatedSpan).op).toEqual('http.server');
expect(spanToJSON(generatedSpan).description).toEqual('POST /');
});

test('continues a trace', async () => {
Expand All @@ -70,55 +90,93 @@ describe('Bun Serve Integration', () => {
const PARENT_SAMPLED = '1';

const SENTRY_TRACE_HEADER = `${TRACE_ID}-${PARENT_SPAN_ID}-${PARENT_SAMPLED}`;
const SENTRY_BAGGAGE_HEADER = 'sentry-version=1.0,sentry-environment=production';
const SENTRY_BAGGAGE_HEADER = 'sentry-version=1.0,sentry-sample_rand=0.42,sentry-environment=production';

client.on('spanEnd', span => {
expect(span.spanContext().traceId).toBe(TRACE_ID);
expect(spanToJSON(span).parent_span_id).toBe(PARENT_SPAN_ID);
expect(spanIsSampled(span)).toBe(true);
expect(span.isRecording()).toBe(false);
let generatedSpan: Span | undefined;

expect(getDynamicSamplingContextFromSpan(span)).toStrictEqual({
version: '1.0',
environment: 'production',
});
client.on('spanEnd', span => {
generatedSpan = span;
});

const server = Bun.serve({
async fetch(_req) {
return new Response('Bun!');
},
port: DEFAULT_PORT,
port,
});

await fetch('http://localhost:22114/', {
await fetch(`http://localhost:${port}/`, {
headers: { 'sentry-trace': SENTRY_TRACE_HEADER, baggage: SENTRY_BAGGAGE_HEADER },
});

server.stop();

if (!generatedSpan) {
throw 'No span was generated in the test';
}

expect(generatedSpan.spanContext().traceId).toBe(TRACE_ID);
expect(spanToJSON(generatedSpan).parent_span_id).toBe(PARENT_SPAN_ID);
expect(spanIsSampled(generatedSpan)).toBe(true);
expect(generatedSpan.isRecording()).toBe(false);

expect(getDynamicSamplingContextFromSpan(generatedSpan)).toStrictEqual({
version: '1.0',
sample_rand: '0.42',
environment: 'production',
});
});

test('does not create transactions for OPTIONS or HEAD requests', async () => {
client.on('spanEnd', () => {
// This will never run, but we want to make sure it doesn't run.
expect(false).toEqual(true);
let generatedSpan: Span | undefined;

client.on('spanEnd', span => {
generatedSpan = span;
});

const server = Bun.serve({
async fetch(_req) {
return new Response('Bun!');
},
port: DEFAULT_PORT,
port,
});

await fetch('http://localhost:22114/', {
await fetch(`http://localhost:${port}/`, {
method: 'OPTIONS',
});

await fetch('http://localhost:22114/', {
await fetch(`http://localhost:${port}/`, {
method: 'HEAD',
});

server.stop();

expect(generatedSpan).toBeUndefined();
});

test('intruments the server again if it is reloaded', async () => {
let serverWasInstrumented = false;
client.on('spanEnd', () => {
serverWasInstrumented = true;
});

const server = Bun.serve({
async fetch(_req) {
return new Response('Bun!');
},
port,
});

server.reload({
async fetch(_req) {
return new Response('Reloaded Bun!');
},
});

await fetch(`http://localhost:${port}/`);

server.stop();

expect(serverWasInstrumented).toBeTrue();
});
});
20 changes: 13 additions & 7 deletions packages/bun/test/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { expect, test } from 'bun:test';
import { describe, expect, test } from 'bun:test';

import { init } from '../src/index';

test("calling init shouldn't fail", () => {
init({
describe('Bun SDK', () => {
const initOptions = {
dsn: 'https://[email protected]/0000000',
tracesSampleRate: 1,
};

Comment on lines +6 to +9
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out that the SDK initialised in here is effecting the SDK being used by bunserver.test.ts above. I am not familiar enough with the Sentry codebase to know how to properly reset after these tests but for sure something is missing here as the tests are not properly isolated.

I've had to add tracesSampleRate: 1 for now to ensure that the SDK being reused doesn't totally break the other tests which depend on tracing happening for every single call.

test("calling init shouldn't fail", () => {
expect(() => {
init(initOptions);
}).not.toThrow();
});
expect(true).toBe(true);
});

test('should return client from init', () => {
expect(init({})).not.toBeUndefined();
test('should return client from init', () => {
expect(init(initOptions)).not.toBeUndefined();
});
});
Loading