diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-ignoreConnect.js b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-ignoreConnect.js new file mode 100644 index 000000000000..1de77ec5b89c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/scenario-ignoreConnect.js @@ -0,0 +1,46 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.postgresIntegration({ ignoreConnectSpans: true })], + transport: loggingTransport, +}); + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +const { Client } = require('pg'); + +const client = new Client({ port: 5494, user: 'test', password: 'test', database: 'tests' }); + +async function run() { + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async () => { + try { + await client.connect(); + + await client + .query( + 'CREATE TABLE "User" ("id" SERIAL NOT NULL,"createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,"email" TEXT NOT NULL,"name" TEXT,CONSTRAINT "User_pkey" PRIMARY KEY ("id"));', + ) + .catch(() => { + // if this is not a fresh database, the table might already exist + }); + + await client.query('INSERT INTO "User" ("email", "name") VALUES ($1, $2)', ['tim', 'tim@domain.com']); + await client.query('SELECT * FROM "User"'); + } finally { + await client.end(); + } + }, + ); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts index b0d0649a1ac9..1fd03e92d0e2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts @@ -57,6 +57,54 @@ describe('postgres auto instrumentation', () => { .completed(); }); + test("doesn't emit connect spans if ignoreConnectSpans is true", { timeout: 90_000 }, async () => { + await createRunner(__dirname, 'scenario-ignoreConnect.js') + .withDockerCompose({ + workingDirectory: [__dirname], + readyMatches: ['port 5432'], + setupCommand: 'yarn', + }) + .expect({ + transaction: txn => { + const spanNames = txn.spans?.map(span => span.description); + expect(spanNames?.find(name => name?.includes('connect'))).toBeUndefined(); + expect(txn).toMatchObject({ + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'INSERT INTO "User" ("email", "name") VALUES ($1, $2)', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'INSERT INTO "User" ("email", "name") VALUES ($1, $2)', + op: 'db', + status: 'ok', + origin: 'auto.db.otel.postgres', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'db.system': 'postgresql', + 'db.name': 'tests', + 'db.statement': 'SELECT * FROM "User"', + 'sentry.origin': 'auto.db.otel.postgres', + 'sentry.op': 'db', + }), + description: 'SELECT * FROM "User"', + op: 'db', + status: 'ok', + origin: 'auto.db.otel.postgres', + }), + ]), + }); + }, + }) + .start() + .completed(); + }); + test('should auto-instrument `pg-native` package', { timeout: 90_000 }, async () => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', diff --git a/packages/node/src/integrations/tracing/postgres.ts b/packages/node/src/integrations/tracing/postgres.ts index d3b3c0cc0edf..7aeb6c671b63 100644 --- a/packages/node/src/integrations/tracing/postgres.ts +++ b/packages/node/src/integrations/tracing/postgres.ts @@ -3,24 +3,29 @@ import type { IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; import { addOriginToSpan, generateInstrumentOnce } from '@sentry/node-core'; +interface PostgresIntegrationOptions { + ignoreConnectSpans?: boolean; +} + const INTEGRATION_NAME = 'Postgres'; export const instrumentPostgres = generateInstrumentOnce( INTEGRATION_NAME, - () => - new PgInstrumentation({ - requireParentSpan: true, - requestHook(span) { - addOriginToSpan(span, 'auto.db.otel.postgres'); - }, - }), + PgInstrumentation, + (options?: PostgresIntegrationOptions) => ({ + requireParentSpan: true, + requestHook(span) { + addOriginToSpan(span, 'auto.db.otel.postgres'); + }, + ignoreConnectSpans: options?.ignoreConnectSpans ?? false, + }), ); -const _postgresIntegration = (() => { +const _postgresIntegration = ((options?: PostgresIntegrationOptions) => { return { name: INTEGRATION_NAME, setupOnce() { - instrumentPostgres(); + instrumentPostgres(options); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/test/integrations/tracing/postgres.test.ts b/packages/node/test/integrations/tracing/postgres.test.ts new file mode 100644 index 000000000000..aeb72e75c498 --- /dev/null +++ b/packages/node/test/integrations/tracing/postgres.test.ts @@ -0,0 +1,93 @@ +import { PgInstrumentation } from '@opentelemetry/instrumentation-pg'; +import { INSTRUMENTED } from '@sentry/node-core'; +import { beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest'; +import { instrumentPostgres, postgresIntegration } from '../../../src/integrations/tracing/postgres'; + +vi.mock('@opentelemetry/instrumentation-pg'); + +describe('postgres integration', () => { + beforeEach(() => { + vi.clearAllMocks(); + delete INSTRUMENTED.Postgres; + + (PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({ + setTracerProvider: () => undefined, + setMeterProvider: () => undefined, + getConfig: () => ({}), + setConfig: () => ({}), + enable: () => undefined, + })); + }); + + it('has a name and setupOnce method', () => { + const integration = postgresIntegration(); + expect(integration.name).toBe('Postgres'); + expect(typeof integration.setupOnce).toBe('function'); + }); + + it('passes ignoreConnectSpans: true to PgInstrumentation when set on integration', () => { + postgresIntegration({ ignoreConnectSpans: true }).setupOnce!(); + + expect(PgInstrumentation).toHaveBeenCalledTimes(1); + expect(PgInstrumentation).toHaveBeenCalledWith({ + requireParentSpan: true, + requestHook: expect.any(Function), + ignoreConnectSpans: true, + }); + }); + + it('passes ignoreConnectSpans: false to PgInstrumentation by default', () => { + postgresIntegration().setupOnce!(); + + expect(PgInstrumentation).toHaveBeenCalledTimes(1); + expect(PgInstrumentation).toHaveBeenCalledWith({ + requireParentSpan: true, + requestHook: expect.any(Function), + ignoreConnectSpans: false, + }); + }); + + it('instrumentPostgres receives ignoreConnectSpans option', () => { + instrumentPostgres({ ignoreConnectSpans: true }); + + expect(PgInstrumentation).toHaveBeenCalledTimes(1); + expect(PgInstrumentation).toHaveBeenCalledWith({ + requireParentSpan: true, + requestHook: expect.any(Function), + ignoreConnectSpans: true, + }); + }); + + it('second call to instrumentPostgres passes full config to setConfig, not raw user options', () => { + const mockSetConfig = vi.fn(); + (PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({ + setTracerProvider: () => undefined, + setMeterProvider: () => undefined, + getConfig: () => ({}), + setConfig: mockSetConfig, + enable: () => undefined, + })); + + instrumentPostgres({ ignoreConnectSpans: true }); + expect(PgInstrumentation).toHaveBeenCalledWith( + expect.objectContaining({ + requireParentSpan: true, + ignoreConnectSpans: true, + requestHook: expect.any(Function), + }), + ); + + mockSetConfig.mockClear(); + instrumentPostgres({ ignoreConnectSpans: false }); + + expect(PgInstrumentation).toHaveBeenCalledTimes(1); + expect(mockSetConfig).toHaveBeenCalledTimes(1); + expect(mockSetConfig).toHaveBeenCalledWith( + expect.objectContaining({ + requireParentSpan: true, + ignoreConnectSpans: false, + requestHook: expect.any(Function), + }), + ); + }); +});