Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/instrumentation-pg/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ PostgreSQL instrumentation has few options available to choose from. You can set
| `responseHook` | `PgInstrumentationExecutionResponseHook` (function) | Function for adding custom span attributes from db response |
| `requireParentSpan` | `boolean` | If true, requires a parent span to create new spans (default false) |
| `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context (default false). _NOTE: A comment will not be added to queries that already contain `--` or `/* ... */` in them, even if these are not actually part of comments_ |
| `skipConnectSpans` | `boolean` | If true, `pg.connect` and `pg-pool.connect` spans will not be created. Query spans and pool metrics are still recorded (default false) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `skipConnectSpans` | `boolean` | If true, `pg.connect` and `pg-pool.connect` spans will not be created. Query spans and pool metrics are still recorded (default false) |
| `ignoreConnectSpans` | `boolean` | If true, `pg.connect` and `pg-pool.connect` spans will not be created. Query spans and pool metrics are still recorded (default false) |


## Semantic Conventions

Expand Down
17 changes: 13 additions & 4 deletions packages/instrumentation-pg/src/instrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
const plugin = this;
return (original: PgClientConnect) => {
return function connect(this: pgTypes.Client, callback?: Function) {
if (utils.shouldSkipInstrumentation(plugin.getConfig())) {
const config = plugin.getConfig();

if (utils.shouldSkipInstrumentation(config) || config.skipConnectSpans) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (utils.shouldSkipInstrumentation(config) || config.skipConnectSpans) {
if (utils.shouldSkipInstrumentation(config) || config.ignoreConnectSpans) {

return original.call(this, callback);
}

Expand Down Expand Up @@ -574,7 +576,16 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
const plugin = this;
return (originalConnect: typeof pgPoolTypes.prototype.connect) => {
return function connect(this: PgPoolExtended, callback?: PgPoolCallback) {
if (utils.shouldSkipInstrumentation(plugin.getConfig())) {
const config = plugin.getConfig();

if (utils.shouldSkipInstrumentation(config)) {
return originalConnect.call(this, callback as any);
}

// Still set up event listeners for metrics even when skipping spans
plugin._setPoolConnectEventListeners(this);

if (config.skipConnectSpans) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (config.skipConnectSpans) {
if (config.ignoreConnectSpans) {

return originalConnect.call(this, callback as any);
}

Expand All @@ -587,8 +598,6 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
),
});

plugin._setPoolConnectEventListeners(this);

if (callback) {
const parentSpan = trace.getSpan(context.active());
callback = utils.patchCallbackPGPool(
Expand Down
8 changes: 8 additions & 0 deletions packages/instrumentation-pg/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,12 @@ export interface PgInstrumentationConfig extends InstrumentationConfig {
* the tracing context, following the {@link https://github.com/open-telemetry/opentelemetry-sqlcommenter sqlcommenter} format
*/
addSqlCommenterCommentToQueries?: boolean;

/**
* If true, `pg.connect` and `pg-pool.connect` spans will not be created.
* Query spans and pool metrics are still recorded.
*
* @default false
*/
skipConnectSpans?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
skipConnectSpans?: boolean;
ignoreConnectSpans?: boolean;

}
61 changes: 61 additions & 0 deletions packages/instrumentation-pg/test/pg-pool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,67 @@ describe('pg-pool', () => {
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 0);
});

it('should not create connect spans when skipConnectSpans=true', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should not create connect spans when skipConnectSpans=true', async () => {
it('should not create connect spans when ignoreConnectSpans=true', async () => {

const newPool = new pgPool(CONFIG);
create({
skipConnectSpans: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
skipConnectSpans: true,
ignoreConnectSpans: true,

});
const span = provider.getTracer('test-pg-pool').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
const client = await newPool.connect();
try {
await client.query('SELECT NOW()');
} finally {
client.release();
}
});
await newPool.end();

const spans = memoryExporter.getFinishedSpans();
const poolConnectSpans = spans.filter(s => s.name === 'pg-pool.connect');
const clientConnectSpans = spans.filter(s => s.name === 'pg.connect');
const querySpans = spans.filter(s => s.name.startsWith('pg.query'));

assert.strictEqual(
poolConnectSpans.length,
0,
'Expected no pg-pool.connect spans'
);
assert.strictEqual(
clientConnectSpans.length,
0,
'Expected no pg.connect spans'
);
assert.ok(querySpans.length > 0, 'Expected query spans to be created');
});

it('should still record pool metrics when skipConnectSpans=true', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should still record pool metrics when skipConnectSpans=true', async () => {
it('should still record pool metrics when ignoreConnectSpans=true', async () => {

const metricReader = testUtils.initMeterProvider(instrumentation);
const newPool = new pgPool(CONFIG);
create({
skipConnectSpans: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
skipConnectSpans: true,
ignoreConnectSpans: true,

});

const client = await newPool.connect();
try {
await client.query('SELECT NOW()');
} finally {
client.release();
}

const { resourceMetrics, errors } = await metricReader.collect();
assert.deepEqual(errors, [], 'expected no errors during metric collection');

const metrics = resourceMetrics.scopeMetrics[0].metrics;
assert.strictEqual(
metrics[1].descriptor.name,
METRIC_DB_CLIENT_CONNECTION_COUNT,
'Expected connection count metric'
);

await newPool.end();
});
});

describe('#pool.query()', () => {
Expand Down
Loading