-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(bun,deno,node): pg orchestrion instrumentation #21826
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
75fa393
910fdfe
e70e4a8
dea56c0
1fa0663
f0f382e
a72910a
81a297d
2c2d110
972275c
891901c
b23de23
87f9faf
373c014
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 |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Builds the smoke scenario with the orchestrion `bun build` plugin and writes | ||
| // the bundle to a temp dir, printing the output path for test.ts to execute. | ||
| // | ||
| // A successful build proves `bun build` runs with the plugin; running the | ||
| // bundle (see test.ts) then proves the bundled `pg` is actually instrumented. | ||
|
|
||
| // @ts-ignore -- subpath export resolved by Bun at runtime; the package | ||
| // tsconfig's node module resolution can't see `exports` subpaths. | ||
| import { sentryBunPlugin } from '@sentry/bun/plugin'; | ||
| import { tmpdir } from 'os'; | ||
| import { join } from 'path'; | ||
|
|
||
| void (async () => { | ||
| const outdir = join(tmpdir(), `sentry-bun-orchestrion-pg-${process.pid}-${Date.now()}`); | ||
| const result = await Bun.build({ | ||
| entrypoints: [join(__dirname, 'scenario.ts')], | ||
| target: 'bun', | ||
| outdir, | ||
| // Deliberately mark `pg` external. An externalized dependency is resolved | ||
| // from `node_modules` at runtime and never passes through the transform's | ||
| // `onLoad`, so its channel injection would be silently skipped. The plugin | ||
| // must strip instrumented packages back out of `external` so they get | ||
| // bundled (and thus transformed). | ||
| external: ['pg'], | ||
|
Member
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. is there a way to automate this similar to how we do this in vite, so users do not need to configure this? |
||
| plugins: [sentryBunPlugin()], | ||
| }); | ||
|
|
||
| if (!result.success) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('BUILD_FAILED', result.logs); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const output = result.outputs[0]; | ||
| if (!output) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('BUILD_FAILED no outputs'); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // eslint-disable-next-line no-console | ||
| console.log(`BUILD_OK outfile=${output.path}`); | ||
| })(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // Bundled entry for the `bun build` smoke test. | ||
| // | ||
| // Once `Bun.build` (with the orchestrion plugin) has transformed `pg`, | ||
| // calling `client.query()` publishes to the `orchestrion:pg:query` tracing | ||
| // channel. | ||
| // | ||
| // `start` fires synchronously on the call, so no live database is needed. | ||
| // | ||
| // We subscribe, run a query, and report which channel events fired | ||
| // (plus the detection marker the plugin's banner sets at boot). | ||
|
|
||
| import { tracingChannel } from 'node:diagnostics_channel'; | ||
|
|
||
| // @ts-ignore -- only the runtime value is needed; pg's types are irrelevant | ||
| import pg from 'pg'; | ||
|
|
||
| interface QueryContext { | ||
| arguments?: unknown[]; | ||
| } | ||
| interface Client { | ||
| query(sql: string, cb: () => void): void; | ||
| } | ||
| interface PgModule { | ||
| Client: new (opts: { host: string; user: string; database: string }) => Client; | ||
| } | ||
|
|
||
| const events: string[] = []; | ||
| let statement = ''; | ||
|
|
||
| tracingChannel('orchestrion:pg:query').subscribe({ | ||
| start(message: unknown) { | ||
| events.push('start'); | ||
| const first = (message as QueryContext).arguments?.[0]; | ||
| statement = typeof first === 'string' ? first : ''; | ||
| }, | ||
| end() { | ||
| events.push('end'); | ||
| }, | ||
| asyncStart() {}, | ||
| asyncEnd() { | ||
| events.push('asyncEnd'); | ||
| }, | ||
| error() {}, | ||
| }); | ||
|
|
||
| const client = new (pg as PgModule).Client({ host: '127.0.0.1', user: 'root', database: 'mydb' }); | ||
| try { | ||
| client.query('SELECT 1 AS solution', () => {}); | ||
| } catch { | ||
| // No live server | ||
| // `start` has already published synchronously by this point. | ||
| } | ||
|
|
||
| const marker = (globalThis as { __SENTRY_ORCHESTRION__?: { runtime?: boolean; bundler?: boolean } }) | ||
| .__SENTRY_ORCHESTRION__; | ||
|
|
||
| setTimeout(() => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`SCENARIO events=${events.join(',')} statement=${statement} marker=${JSON.stringify(marker ?? null)}`); | ||
| process.exit(0); | ||
| }, 200); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { spawnSync } from 'child_process'; | ||
| import { rmSync } from 'fs'; | ||
| import { dirname, join } from 'path'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| const dir = __dirname; | ||
|
|
||
| // Cap each `bun` subprocess. The test runs two of them sequentially, so its | ||
| // own timeout must exceed `2 * SUBPROCESS_TIMEOUT_MS` otherwise the suite's | ||
| // default `testTimeout` (20s) fails the test before these caps do, | ||
| // for example on a slow CI runner where the build+run can take >20s. | ||
| const SUBPROCESS_TIMEOUT_MS = 60_000; | ||
|
|
||
| function runBun(args: string[]): { stdout: string; stderr: string; status: number | null } { | ||
| const res = spawnSync('bun', args, { cwd: dir, encoding: 'utf8', timeout: SUBPROCESS_TIMEOUT_MS }); | ||
| return { stdout: res.stdout ?? '', stderr: res.stderr ?? '', status: res.status }; | ||
| } | ||
|
|
||
| // Bun orchestrion instrumentation is BUILD-ONLY (`@sentry/bun/plugin` is a | ||
| // `Bun.build` plugin; there is no `bun run` preload). | ||
| // | ||
| // A `bun run` runtime plugin cannot instrument CommonJS dependencies like | ||
| // `pg`: any module returned by a runtime `onLoad` plugin in Bun loses its | ||
| // CommonJS named exports | ||
| // | ||
| // When https://github.com/oven-sh/bun/pull/31770 lands, we can revisit an | ||
| // auto-load plugin for `bun run`. | ||
| describe('orchestrion pg instrumentation (Bun)', () => { | ||
| it( | ||
| 'bundles `pg` with the plugin, and the built output fires the pg channel when run', | ||
| () => { | ||
| // Build the scenario with the orchestrion `bun build` plugin. | ||
| const build = runBun(['run', join(dir, 'build.ts')]); | ||
| expect(build.status, `build failed:\nstderr:\n${build.stderr}\nstdout:\n${build.stdout}`).toBe(0); | ||
|
|
||
| const outfile = build.stdout.match(/BUILD_OK outfile=(.+)/)?.[1]?.trim(); | ||
| expect(outfile, `no outfile in build output:\n${build.stdout}`).toBeTruthy(); | ||
|
|
||
| try { | ||
| // Run the built bundle. The bundled (transformed) `pg` should publish | ||
| // to the `orchestrion:pg:query` channel when `client.query()` is | ||
| // called, and the plugin's banner should set the `bundler` marker at | ||
| // boot. | ||
| const run = runBun(['run', outfile as string]); | ||
| expect(run.status, `run failed:\nstderr:\n${run.stderr}\nstdout:\n${run.stdout}`).toBe(0); | ||
|
|
||
| const line = run.stdout.split('\n').find(l => l.startsWith('SCENARIO')) ?? ''; | ||
| // channel `start` fired on `client.query()` | ||
| expect(line).toContain('events=start'); | ||
| // with the expected SQL | ||
| expect(line).toContain('statement=SELECT 1 AS solution'); | ||
| // injected banner ran at bundle boot | ||
| expect(line).toContain('"bundler":true'); | ||
| } finally { | ||
| if (outfile) { | ||
| rmSync(dirname(outfile), { recursive: true, force: true }); | ||
| } | ||
| } | ||
| // Allow for both sequential `runBun` calls hitting their subprocess | ||
| // cap, so the `spawnSync` timeouts (not the vitest 20s def) are the | ||
| // binding limit. | ||
| }, | ||
| 2 * SUBPROCESS_TIMEOUT_MS, | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "imports": { | ||
| "@sentry/deno": "npm:@sentry/deno", | ||
| "pg": "npm:pg@8.16.0" | ||
| }, | ||
| "nodeModulesDir": "manual" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| services: | ||
| db: | ||
| image: postgres:13 | ||
| restart: always | ||
| container_name: e2e-tests-deno-pg | ||
| ports: | ||
| - '5432:5432' | ||
| environment: | ||
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: password | ||
| POSTGRES_DB: postgres | ||
| healthcheck: | ||
| test: ['CMD-SHELL', 'pg_isready -U postgres -d postgres'] | ||
| interval: 2s | ||
| timeout: 3s | ||
| retries: 30 | ||
| start_period: 5s |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { dirname } from 'path'; | ||
| import { fileURLToPath } from 'url'; | ||
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
|
|
||
| export default async function globalSetup() { | ||
| // Start PostgreSQL via Docker Compose. `--wait` blocks until the healthcheck | ||
| // in docker-compose.yml passes, so the Deno app can connect immediately. | ||
| execSync('docker compose up -d --wait', { | ||
| cwd: __dirname, | ||
| stdio: 'inherit', | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { dirname } from 'path'; | ||
| import { fileURLToPath } from 'url'; | ||
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
|
|
||
| export default async function globalTeardown() { | ||
| execSync('docker compose down --volumes', { | ||
| cwd: __dirname, | ||
| stdio: 'inherit', | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| { | ||
|
Member
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. I would not block this but would really ask if this is the way to go with testing this - following this logic we would have so many deno e2e test apps for all kinds of instrumentation 😅 can we not integration test this somehow? |
||
| "name": "deno-pg", | ||
| "version": "1.0.0", | ||
| "private": true, | ||
| "scripts": { | ||
| "start": "docker compose up -d --wait && deno run --allow-net --allow-env --allow-read --allow-sys --allow-write src/app.ts", | ||
| "test": "playwright test", | ||
| "clean": "npx rimraf node_modules pnpm-lock.yaml", | ||
| "test:build": "pnpm install", | ||
| "test:assert": "pnpm test" | ||
| }, | ||
| "dependencies": { | ||
| "@sentry/deno": "file:../../packed/sentry-deno-packed.tgz", | ||
| "pg": "8.16.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@playwright/test": "~1.56.0", | ||
| "@sentry-internal/test-utils": "link:../../../test-utils" | ||
| }, | ||
| "volta": { | ||
| "extends": "../../package.json" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { getPlaywrightConfig } from '@sentry-internal/test-utils'; | ||
|
|
||
| const config = getPlaywrightConfig({ | ||
| startCommand: `pnpm start`, | ||
| port: 3030, | ||
| }); | ||
|
|
||
| export default { | ||
| ...config, | ||
| globalSetup: './global-setup.mjs', | ||
| globalTeardown: './global-teardown.mjs', | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // `@sentry/deno/import` MUST be the very first import: it registers the | ||
| // orchestrion runtime hook, which transforms `pg` (imported dynamically below) | ||
| // to publish the `orchestrion:pg:query` diagnostics channel. | ||
| // In Deno 2.8.0–2.8.2 the hook only works as the first import in the entry | ||
| // graph. | ||
| import '@sentry/deno/import'; | ||
| import * as Sentry from '@sentry/deno'; | ||
|
|
||
| Sentry.init({ | ||
| environment: 'qa', | ||
| dsn: Deno.env.get('E2E_TEST_DSN'), | ||
| debug: !!Deno.env.get('DEBUG'), | ||
| tunnel: 'http://localhost:3031/', // proxy server | ||
| tracesSampleRate: 1, | ||
| }); | ||
|
|
||
| // Dynamic import AFTER init so the orchestrion hook (registered above) is in | ||
| // place to transform `pg/lib/client.js`'s `query`, and so | ||
| // `denoPostgresIntegration` (wired by `init()`) is already subscribed. | ||
| const { default: pg } = await import('pg'); | ||
|
|
||
| const client = new pg.Client({ | ||
| host: Deno.env.get('PGHOST') ?? '127.0.0.1', | ||
| port: Number(Deno.env.get('PGPORT') ?? 5432), | ||
| user: 'postgres', | ||
| password: 'password', | ||
| database: 'postgres', | ||
| }); | ||
|
|
||
| // Swallow connection errors (e.g. the DB container going away at teardown) so | ||
| // they don't become an uncaught exception that crashes the process on | ||
| // shutdown. | ||
| client.on('error', (err: unknown) => { | ||
| // eslint-disable-next-line no-console | ||
| console.error('pg client error', err); | ||
| }); | ||
|
|
||
| client.connect((err: unknown) => { | ||
| if (err) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('pg connect error', err); | ||
| } | ||
| }); | ||
|
|
||
| const port = 3030; | ||
|
|
||
| Deno.serve({ port, hostname: '0.0.0.0' }, async (req: Request) => { | ||
| const url = new URL(req.url); | ||
|
|
||
| // Runs two queries, the second NESTED inside the first's callback. pg | ||
| // dispatches that callback from its socket data handler (a fresh async | ||
| // context), so the nested query's span only lands on this request's | ||
| // http.server transaction if `denoPostgresIntegration`'s AsyncLocalStorage | ||
| // context strategy restored the parent across the async boundary. | ||
| if (url.pathname === '/test-pg') { | ||
| await new Promise<void>((resolve, reject) => { | ||
| client.query('SELECT 1 + 1 AS solution', (err: unknown) => { | ||
| if (err) return reject(err); | ||
| client.query('SELECT NOW()', (err2: unknown) => { | ||
| if (err2) return reject(err2); | ||
| resolve(); | ||
| }); | ||
| }); | ||
| }); | ||
| return Response.json({ status: 'ok' }); | ||
| } | ||
|
|
||
| return new Response('Not found', { status: 404 }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| import { startEventProxyServer } from '@sentry-internal/test-utils'; | ||
|
|
||
| startEventProxyServer({ | ||
| port: 3031, | ||
| proxyServerName: 'deno-pg', | ||
| }); |
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.
q: I wonder if using
startsWith(matrix.test-application, 'deno-')would be better at this pointThere 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.
or just
contains('deno-')?