diff --git a/packages/node/package.json b/packages/node/package.json index 655cb05fad08..f9dbdc9fa334 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -31,7 +31,8 @@ "@types/lru-cache": "^5.1.0", "@types/node": "~10.17.0", "express": "^4.17.1", - "nock": "^13.0.5" + "nock": "^13.0.5", + "undici": "^5.21.0" }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 8edc5e326cea..11549b5d75dc 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -33,6 +33,10 @@ const DEFAULT_UNDICI_OPTIONS: UndiciOptions = { breadcrumbs: true, }; +// Please note that you cannot use `console.log` to debug the callbacks registered to the `diagnostics_channel` API. +// To debug, you can use `writeFileSync` to write to a file: +// https://nodejs.org/api/async_hooks.html#printing-in-asynchook-callbacks + /** * Instruments outgoing HTTP requests made with the `undici` package via * Node's `diagnostics_channel` API. @@ -89,7 +93,7 @@ export class Undici implements Integration { const url = new URL(request.path, request.origin); const stringUrl = url.toString(); - if (isSentryRequest(stringUrl)) { + if (isSentryRequest(stringUrl) || request.__sentry__ !== undefined) { return; } @@ -132,7 +136,6 @@ export class Undici implements Integration { : true; if (shouldPropagate) { - // TODO: Only do this based on tracePropagationTargets request.addHeader('sentry-trace', span.toTraceparent()); if (span.transaction) { const dynamicSamplingContext = span.transaction.getDynamicSamplingContext(); diff --git a/packages/node/src/integrations/undici/types.ts b/packages/node/src/integrations/undici/types.ts index 9cbafeacfbb4..c2d2db125195 100644 --- a/packages/node/src/integrations/undici/types.ts +++ b/packages/node/src/integrations/undici/types.ts @@ -20,7 +20,7 @@ import type { Span } from '@sentry/core'; // Vendored code starts here: -type ChannelListener = (message: unknown, name: string | symbol) => void; +export type ChannelListener = (message: unknown, name: string | symbol) => void; /** * The `diagnostics_channel` module provides an API to create named channels diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts new file mode 100644 index 000000000000..d16a18497e0a --- /dev/null +++ b/packages/node/test/integrations/undici.test.ts @@ -0,0 +1,320 @@ +import type { Transaction } from '@sentry/core'; +import { Hub, makeMain } from '@sentry/core'; +import * as http from 'http'; +import type { fetch as FetchType } from 'undici'; + +import { NodeClient } from '../../src/client'; +import { Undici } from '../../src/integrations/undici'; +import { getDefaultNodeClientOptions } from '../helper/node-client-options'; +import { conditionalTest } from '../utils'; + +const SENTRY_DSN = 'https://0@0.ingest.sentry.io/0'; + +let hub: Hub; +let fetch: typeof FetchType; + +beforeAll(async () => { + await setupTestServer(); + try { + // need to conditionally require `undici` because it's not available in Node 10 + // eslint-disable-next-line @typescript-eslint/no-var-requires + fetch = require('undici').fetch; + } catch (e) { + // eslint-disable-next-line no-console + console.warn('Undici integration tests are skipped because undici is not installed.'); + } +}); + +const DEFAULT_OPTIONS = getDefaultNodeClientOptions({ + dsn: SENTRY_DSN, + tracesSampleRate: 1, + integrations: [new Undici()], +}); + +beforeEach(() => { + const client = new NodeClient(DEFAULT_OPTIONS); + hub = new Hub(client); + makeMain(hub); +}); + +afterEach(() => { + requestHeaders = {}; + setTestServerOptions({ statusCode: 200 }); +}); + +afterAll(() => { + getTestServer()?.close(); +}); + +conditionalTest({ min: 16 })('Undici integration', () => { + it.each([ + [ + 'simple url', + 'http://localhost:18099', + undefined, + { + description: 'GET http://localhost:18099/', + op: 'http.client', + }, + ], + [ + 'url with query', + 'http://localhost:18099?foo=bar', + undefined, + { + description: 'GET http://localhost:18099/', + op: 'http.client', + data: { + 'http.query': '?foo=bar', + }, + }, + ], + [ + 'url with POST method', + 'http://localhost:18099', + { method: 'POST' }, + { + description: 'POST http://localhost:18099/', + }, + ], + [ + 'url with POST method', + 'http://localhost:18099', + { method: 'POST' }, + { + description: 'POST http://localhost:18099/', + }, + ], + [ + 'url with GET as default', + 'http://localhost:18099', + { method: undefined }, + { + description: 'GET http://localhost:18099/', + }, + ], + ])('creates a span with a %s', async (_: string, request, requestInit, expected) => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + await fetch(request, requestInit); + + expect(transaction.spanRecorder?.spans.length).toBe(2); + + const span = transaction.spanRecorder?.spans[1]; + expect(span).toEqual(expect.objectContaining(expected)); + }); + + it('creates a span with internal errors', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + try { + await fetch('http://a-url-that-no-exists.com'); + } catch (e) { + // ignore + } + + expect(transaction.spanRecorder?.spans.length).toBe(2); + + const span = transaction.spanRecorder?.spans[1]; + expect(span).toEqual(expect.objectContaining({ status: 'internal_error' })); + }); + + it('does not create a span for sentry requests', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + try { + await fetch(`${SENTRY_DSN}/sub/route`, { + method: 'POST', + }); + } catch (e) { + // ignore + } + + expect(transaction.spanRecorder?.spans.length).toBe(1); + }); + + it('does not create a span if there is no active spans', async () => { + try { + await fetch(`${SENTRY_DSN}/sub/route`, { method: 'POST' }); + } catch (e) { + // ignore + } + + expect(hub.getScope().getSpan()).toBeUndefined(); + }); + + it('does create a span if `shouldCreateSpanForRequest` is defined', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + const client = new NodeClient({ ...DEFAULT_OPTIONS, shouldCreateSpanForRequest: url => url.includes('yes') }); + hub.bindClient(client); + + await fetch('http://localhost:18099/no', { method: 'POST' }); + + expect(transaction.spanRecorder?.spans.length).toBe(1); + + await fetch('http://localhost:18099/yes', { method: 'POST' }); + + expect(transaction.spanRecorder?.spans.length).toBe(2); + }); + + it('attaches the sentry trace and baggage headers', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + await fetch('http://localhost:18099', { method: 'POST' }); + + expect(transaction.spanRecorder?.spans.length).toBe(2); + const span = transaction.spanRecorder?.spans[1]; + + expect(requestHeaders['sentry-trace']).toEqual(span?.toTraceparent()); + expect(requestHeaders['baggage']).toEqual( + `sentry-environment=production,sentry-transaction=test-transaction,sentry-public_key=0,sentry-trace_id=${transaction.traceId},sentry-sample_rate=1`, + ); + }); + + it('does not attach headers if `shouldCreateSpanForRequest` does not create a span', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + const client = new NodeClient({ ...DEFAULT_OPTIONS, shouldCreateSpanForRequest: url => url.includes('yes') }); + hub.bindClient(client); + + await fetch('http://localhost:18099/no', { method: 'POST' }); + + expect(requestHeaders['sentry-trace']).toBeUndefined(); + expect(requestHeaders['baggage']).toBeUndefined(); + + await fetch('http://localhost:18099/yes', { method: 'POST' }); + + expect(requestHeaders['sentry-trace']).toBeDefined(); + expect(requestHeaders['baggage']).toBeDefined(); + }); + + it('uses tracePropagationTargets', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + const client = new NodeClient({ ...DEFAULT_OPTIONS, tracePropagationTargets: ['/yes'] }); + hub.bindClient(client); + + expect(transaction.spanRecorder?.spans.length).toBe(1); + + await fetch('http://localhost:18099/no', { method: 'POST' }); + + expect(transaction.spanRecorder?.spans.length).toBe(2); + + expect(requestHeaders['sentry-trace']).toBeUndefined(); + expect(requestHeaders['baggage']).toBeUndefined(); + + await fetch('http://localhost:18099/yes', { method: 'POST' }); + + expect(transaction.spanRecorder?.spans.length).toBe(3); + + expect(requestHeaders['sentry-trace']).toBeDefined(); + expect(requestHeaders['baggage']).toBeDefined(); + }); + + it('adds a breadcrumb on request', async () => { + expect.assertions(1); + + const client = new NodeClient({ + ...DEFAULT_OPTIONS, + beforeBreadcrumb: breadcrumb => { + expect(breadcrumb).toEqual({ + category: 'http', + data: { + method: 'POST', + status_code: 200, + url: 'http://localhost:18099/', + }, + type: 'http', + timestamp: expect.any(Number), + }); + return breadcrumb; + }, + }); + hub.bindClient(client); + + await fetch('http://localhost:18099', { method: 'POST' }); + }); + + it('adds a breadcrumb on errored request', async () => { + expect.assertions(1); + + const client = new NodeClient({ + ...DEFAULT_OPTIONS, + beforeBreadcrumb: breadcrumb => { + expect(breadcrumb).toEqual({ + category: 'http', + data: { + method: 'GET', + url: 'http://a-url-that-no-exists.com/', + }, + level: 'error', + type: 'http', + timestamp: expect.any(Number), + }); + return breadcrumb; + }, + }); + hub.bindClient(client); + + try { + await fetch('http://a-url-that-no-exists.com'); + } catch (e) { + // ignore + } + }); +}); + +interface TestServerOptions { + statusCode: number; + responseHeaders?: Record; +} + +let testServer: http.Server | undefined; + +let requestHeaders: any = {}; + +let testServerOptions: TestServerOptions = { + statusCode: 200, +}; + +function setTestServerOptions(options: TestServerOptions): void { + testServerOptions = { ...options }; +} + +function getTestServer(): http.Server | undefined { + return testServer; +} + +function setupTestServer() { + testServer = http.createServer((req, res) => { + const chunks: Buffer[] = []; + + req.on('data', data => { + chunks.push(data); + }); + + req.on('end', () => { + requestHeaders = req.headers; + }); + + res.writeHead(testServerOptions.statusCode, testServerOptions.responseHeaders); + res.end(); + + // also terminate socket because keepalive hangs connection a bit + res.connection.end(); + }); + + testServer.listen(18099, 'localhost'); + + return new Promise(resolve => { + testServer?.on('listening', resolve); + }); +} diff --git a/packages/node/test/utils.ts b/packages/node/test/utils.ts new file mode 100644 index 000000000000..1394183f928d --- /dev/null +++ b/packages/node/test/utils.ts @@ -0,0 +1,18 @@ +import { parseSemver } from '@sentry/utils'; + +/** + * Returns`describe` or `describe.skip` depending on allowed major versions of Node. + * + * @param {{ min?: number; max?: number }} allowedVersion + * @return {*} {jest.Describe} + */ +export const conditionalTest = (allowedVersion: { min?: number; max?: number }): jest.Describe => { + const NODE_VERSION = parseSemver(process.versions.node).major; + if (!NODE_VERSION) { + return describe.skip as jest.Describe; + } + + return NODE_VERSION < (allowedVersion.min || -Infinity) || NODE_VERSION > (allowedVersion.max || Infinity) + ? (describe.skip as jest.Describe) + : (describe as any); +}; diff --git a/yarn.lock b/yarn.lock index 7f4e13e3108b..6726a721b75c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26448,6 +26448,13 @@ undici@5.20.0: dependencies: busboy "^1.6.0" +undici@^5.21.0: + version "5.21.0" + resolved "https://registry.yarnpkg.com/undici/-/undici-5.21.0.tgz#b00dfc381f202565ab7f52023222ab862bb2494f" + integrity sha512-HOjK8l6a57b2ZGXOcUsI5NLfoTrfmbOl90ixJDl0AEFG4wgHNDQxtZy15/ZQp7HhjkpaGlp/eneMgtsu1dIlUA== + dependencies: + busboy "^1.6.0" + unicode-canonical-property-names-ecmascript@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/unicode-canonical-property-names-ecmascript/-/unicode-canonical-property-names-ecmascript-2.0.0.tgz#301acdc525631670d39f6146e0e77ff6bbdebddc"