Skip to content

Commit c1f83b7

Browse files
Lms24Lms24
authored
feat(node): Add ignoreConnectSpans option to postgresIntegration (#19291)
OTel's `PgInstrumentation` exposes an option to ignore `pg(.pool).connect` spans. This option was added recently in open-telemetry/opentelemetry-js-contrib#3280. We should allow users to configure our wrapping `postgresIntegration` with the same option.
1 parent 7946bb8 commit c1f83b7

File tree

4 files changed

+201
-9
lines changed

4 files changed

+201
-9
lines changed

‎dev-packages/node-integration-tests/suites/tracing/postgres/scenario-ignoreConnect.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
integrations: [Sentry.postgresIntegration({ ignoreConnectSpans: true })],
9+
transport: loggingTransport,
10+
});
11+
12+
// Stop the process from exiting before the transaction is sent
13+
setInterval(() => {}, 1000);
14+
15+
const { Client } = require('pg');
16+
17+
const client = new Client({ port: 5494, user: 'test', password: 'test', database: 'tests' });
18+
19+
async function run() {
20+
await Sentry.startSpan(
21+
{
22+
name: 'Test Transaction',
23+
op: 'transaction',
24+
},
25+
async () => {
26+
try {
27+
await client.connect();
28+
29+
await client
30+
.query(
31+
'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"));',
32+
)
33+
.catch(() => {
34+
// if this is not a fresh database, the table might already exist
35+
});
36+
37+
await client.query('INSERT INTO "User" ("email", "name") VALUES ($1, $2)', ['tim', 'tim@domain.com']);
38+
await client.query('SELECT * FROM "User"');
39+
} finally {
40+
await client.end();
41+
}
42+
},
43+
);
44+
}
45+
46+
run();

‎dev-packages/node-integration-tests/suites/tracing/postgres/test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,54 @@ describe('postgres auto instrumentation', () => {
5757
.completed();
5858
});
5959

60+
test("doesn't emit connect spans if ignoreConnectSpans is true", { timeout: 90_000 }, async () => {
61+
await createRunner(__dirname, 'scenario-ignoreConnect.js')
62+
.withDockerCompose({
63+
workingDirectory: [__dirname],
64+
readyMatches: ['port 5432'],
65+
setupCommand: 'yarn',
66+
})
67+
.expect({
68+
transaction: txn => {
69+
const spanNames = txn.spans?.map(span => span.description);
70+
expect(spanNames?.find(name => name?.includes('connect'))).toBeUndefined();
71+
expect(txn).toMatchObject({
72+
transaction: 'Test Transaction',
73+
spans: expect.arrayContaining([
74+
expect.objectContaining({
75+
data: expect.objectContaining({
76+
'db.system': 'postgresql',
77+
'db.name': 'tests',
78+
'db.statement': 'INSERT INTO "User" ("email", "name") VALUES ($1, $2)',
79+
'sentry.origin': 'auto.db.otel.postgres',
80+
'sentry.op': 'db',
81+
}),
82+
description: 'INSERT INTO "User" ("email", "name") VALUES ($1, $2)',
83+
op: 'db',
84+
status: 'ok',
85+
origin: 'auto.db.otel.postgres',
86+
}),
87+
expect.objectContaining({
88+
data: expect.objectContaining({
89+
'db.system': 'postgresql',
90+
'db.name': 'tests',
91+
'db.statement': 'SELECT * FROM "User"',
92+
'sentry.origin': 'auto.db.otel.postgres',
93+
'sentry.op': 'db',
94+
}),
95+
description: 'SELECT * FROM "User"',
96+
op: 'db',
97+
status: 'ok',
98+
origin: 'auto.db.otel.postgres',
99+
}),
100+
]),
101+
});
102+
},
103+
})
104+
.start()
105+
.completed();
106+
});
107+
60108
test('should auto-instrument `pg-native` package', { timeout: 90_000 }, async () => {
61109
const EXPECTED_TRANSACTION = {
62110
transaction: 'Test Transaction',

‎packages/node/src/integrations/tracing/postgres.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,29 @@ import type { IntegrationFn } from '@sentry/core';
33
import { defineIntegration } from '@sentry/core';
44
import { addOriginToSpan, generateInstrumentOnce } from '@sentry/node-core';
55

6+
interface PostgresIntegrationOptions {
7+
ignoreConnectSpans?: boolean;
8+
}
9+
610
const INTEGRATION_NAME = 'Postgres';
711

812
export const instrumentPostgres = generateInstrumentOnce(
913
INTEGRATION_NAME,
10-
() =>
11-
new PgInstrumentation({
12-
requireParentSpan: true,
13-
requestHook(span) {
14-
addOriginToSpan(span, 'auto.db.otel.postgres');
15-
},
16-
}),
14+
PgInstrumentation,
15+
(options?: PostgresIntegrationOptions) => ({
16+
requireParentSpan: true,
17+
requestHook(span) {
18+
addOriginToSpan(span, 'auto.db.otel.postgres');
19+
},
20+
ignoreConnectSpans: options?.ignoreConnectSpans ?? false,
21+
}),
1722
);
1823

19-
const _postgresIntegration = (() => {
24+
const _postgresIntegration = ((options?: PostgresIntegrationOptions) => {
2025
return {
2126
name: INTEGRATION_NAME,
2227
setupOnce() {
23-
instrumentPostgres();
28+
instrumentPostgres(options);
2429
},
2530
};
2631
}) satisfies IntegrationFn;

‎packages/node/test/integrations/tracing/postgres.test.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { PgInstrumentation } from '@opentelemetry/instrumentation-pg';
2+
import { INSTRUMENTED } from '@sentry/node-core';
3+
import { beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest';
4+
import { instrumentPostgres, postgresIntegration } from '../../../src/integrations/tracing/postgres';
5+
6+
vi.mock('@opentelemetry/instrumentation-pg');
7+
8+
describe('postgres integration', () => {
9+
beforeEach(() => {
10+
vi.clearAllMocks();
11+
delete INSTRUMENTED.Postgres;
12+
13+
(PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({
14+
setTracerProvider: () => undefined,
15+
setMeterProvider: () => undefined,
16+
getConfig: () => ({}),
17+
setConfig: () => ({}),
18+
enable: () => undefined,
19+
}));
20+
});
21+
22+
it('has a name and setupOnce method', () => {
23+
const integration = postgresIntegration();
24+
expect(integration.name).toBe('Postgres');
25+
expect(typeof integration.setupOnce).toBe('function');
26+
});
27+
28+
it('passes ignoreConnectSpans: true to PgInstrumentation when set on integration', () => {
29+
postgresIntegration({ ignoreConnectSpans: true }).setupOnce!();
30+
31+
expect(PgInstrumentation).toHaveBeenCalledTimes(1);
32+
expect(PgInstrumentation).toHaveBeenCalledWith({
33+
requireParentSpan: true,
34+
requestHook: expect.any(Function),
35+
ignoreConnectSpans: true,
36+
});
37+
});
38+
39+
it('passes ignoreConnectSpans: false to PgInstrumentation by default', () => {
40+
postgresIntegration().setupOnce!();
41+
42+
expect(PgInstrumentation).toHaveBeenCalledTimes(1);
43+
expect(PgInstrumentation).toHaveBeenCalledWith({
44+
requireParentSpan: true,
45+
requestHook: expect.any(Function),
46+
ignoreConnectSpans: false,
47+
});
48+
});
49+
50+
it('instrumentPostgres receives ignoreConnectSpans option', () => {
51+
instrumentPostgres({ ignoreConnectSpans: true });
52+
53+
expect(PgInstrumentation).toHaveBeenCalledTimes(1);
54+
expect(PgInstrumentation).toHaveBeenCalledWith({
55+
requireParentSpan: true,
56+
requestHook: expect.any(Function),
57+
ignoreConnectSpans: true,
58+
});
59+
});
60+
61+
it('second call to instrumentPostgres passes full config to setConfig, not raw user options', () => {
62+
const mockSetConfig = vi.fn();
63+
(PgInstrumentation as unknown as MockInstance).mockImplementation(() => ({
64+
setTracerProvider: () => undefined,
65+
setMeterProvider: () => undefined,
66+
getConfig: () => ({}),
67+
setConfig: mockSetConfig,
68+
enable: () => undefined,
69+
}));
70+
71+
instrumentPostgres({ ignoreConnectSpans: true });
72+
expect(PgInstrumentation).toHaveBeenCalledWith(
73+
expect.objectContaining({
74+
requireParentSpan: true,
75+
ignoreConnectSpans: true,
76+
requestHook: expect.any(Function),
77+
}),
78+
);
79+
80+
mockSetConfig.mockClear();
81+
instrumentPostgres({ ignoreConnectSpans: false });
82+
83+
expect(PgInstrumentation).toHaveBeenCalledTimes(1);
84+
expect(mockSetConfig).toHaveBeenCalledTimes(1);
85+
expect(mockSetConfig).toHaveBeenCalledWith(
86+
expect.objectContaining({
87+
requireParentSpan: true,
88+
ignoreConnectSpans: false,
89+
requestHook: expect.any(Function),
90+
}),
91+
);
92+
});
93+
});

0 commit comments

Comments
 (0)