-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
// Fun fact: Bun = 2 21 14 :) | ||
let port: number = 22114; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't reuse the port on each start since |
||
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 () => { | ||
|
@@ -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(); | ||
}); | ||
}); |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I've had to add |
||
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(); | ||
}); | ||
}); |
There was a problem hiding this comment.
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.