Skip to content
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

feat(node): Add generic-pool integration #13465

Merged
merged 7 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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 dev-packages/node-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"cors": "^2.8.5",
"cron": "^3.1.6",
"express": "^4.17.3",
"generic-pool": "3.9.0",
"graphql": "^16.3.0",
"http-terminator": "^3.2.0",
"ioredis": "^5.4.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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,
transport: loggingTransport,
});

// Stop the process from exiting before the transaction is sent
setInterval(() => {}, 1000);

const mysql = require('mysql');
Zen-cronic marked this conversation as resolved.
Show resolved Hide resolved
const genericPool = require('generic-pool');

const factory = {
create: function () {
return mysql.createConnection({
user: 'root',
Dismissed Show dismissed Hide dismissed
password: 'docker',
Dismissed Show dismissed Hide dismissed
});
},
destroy: function (client) {
client.end(err => {
if (err) {
// eslint-disable-next-line no-console
console.error('Error while disconnecting MySQL:', err);
}
});
},
};

const opts = {
max: 10,
min: 2,
};

const myPool = genericPool.createPool(factory, opts);

async function run() {
await Sentry.startSpan(
{
op: 'transaction',
name: 'Test Transaction',
},
async () => {
try {
const client1 = await myPool.acquire();
const client2 = await myPool.acquire();

client1.query('SELECT NOW()', function () {
myPool.release(client1);
});

client2.query('SELECT 1 + 1 AS solution', function () {
myPool.release(client2);
});
} catch (err) {
// eslint-disable-next-line no-console
console.error('Error while pooling MySQL:', err);
} finally {
await myPool.drain();
await myPool.clear();
}
},
);
}

// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

describe('genericPool auto instrumentation', () => {
afterAll(() => {
cleanupChildProcesses();
});

test('should auto-instrument `genericPool` package when calling pool.require()', done => {
const EXPECTED_TRANSACTION = {
transaction: 'Test Transaction',
spans: expect.arrayContaining([
expect.objectContaining({
description: 'generic-pool.aquire',
origin: 'auto.db.otel.generic-pool',
data: {
'sentry.origin': 'auto.db.otel.generic-pool',
},
status: 'ok',
}),

expect.objectContaining({
description: 'generic-pool.aquire',
origin: 'auto.db.otel.generic-pool',
data: {
'sentry.origin': 'auto.db.otel.generic-pool',
},
status: 'ok',
}),
]),
};

createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done);
});
});
1 change: 1 addition & 0 deletions packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"@opentelemetry/instrumentation-express": "0.41.1",
"@opentelemetry/instrumentation-fastify": "0.38.0",
"@opentelemetry/instrumentation-fs": "0.14.0",
"@opentelemetry/instrumentation-generic-pool": "0.38.0",
"@opentelemetry/instrumentation-graphql": "0.42.0",
"@opentelemetry/instrumentation-hapi": "0.40.0",
"@opentelemetry/instrumentation-http": "0.52.1",
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export { hapiIntegration, setupHapiErrorHandler } from './integrations/tracing/h
export { koaIntegration, setupKoaErrorHandler } from './integrations/tracing/koa';
export { connectIntegration, setupConnectErrorHandler } from './integrations/tracing/connect';
export { spotlightIntegration } from './integrations/spotlight';
export { genericPoolIntegration } from './integrations/tracing/genericPool';

export { SentryContextManager } from './otel/contextManager';
export { generateInstrumentOnce } from './otel/instrument';
Expand Down
33 changes: 33 additions & 0 deletions packages/node/src/integrations/tracing/genericPool.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { GenericPoolInstrumentation } from '@opentelemetry/instrumentation-generic-pool';
import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanToJSON } from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';
import { generateInstrumentOnce } from '../../otel/instrument';

const INTEGRATION_NAME = 'GenericPool';

export const instrumentGenericPool = generateInstrumentOnce(INTEGRATION_NAME, () => new GenericPoolInstrumentation({}));

const _genericPoolIntegration = (() => {
return {
name: INTEGRATION_NAME,
setupOnce() {
instrumentGenericPool();
},

setup(client) {
client.on('spanStart', span => {
const spanJSON = spanToJSON(span);
if (spanJSON.description === 'generic-pool.aquire') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The description has a typo - missing c in aquire. This is an upstream issue.

Copy link
Contributor Author

@Zen-cronic Zen-cronic Aug 27, 2024

Choose a reason for hiding this comment

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

Fixed in upstream PR. Landed in instrumentation-generic-pool-v0.38.1.

span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.generic-pool');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the docs, trace origin name cannot contain "-".

}
});
},
};
}) satisfies IntegrationFn;

/**
* GenericPool integration
*
* Capture tracing data for GenericPool.
*/
export const genericPoolIntegration = defineIntegration(_genericPoolIntegration);
3 changes: 3 additions & 0 deletions packages/node/src/integrations/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { instrumentMysql2, mysql2Integration } from './mysql2';
import { instrumentNest, nestIntegration } from './nest/nest';
import { instrumentPostgres, postgresIntegration } from './postgres';
import { instrumentRedis, redisIntegration } from './redis';
import { instrumentGenericPool, genericPoolIntegration } from './genericPool'

/**
* With OTEL, all performance integrations will be added, as OTEL only initializes them when the patched package is actually required.
Expand All @@ -37,6 +38,7 @@ export function getAutoPerformanceIntegrations(): Integration[] {
hapiIntegration(),
koaIntegration(),
connectIntegration(),
genericPoolIntegration(),
];
}

Expand All @@ -61,5 +63,6 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) =>
instrumentHapi,
instrumentGraphql,
instrumentRedis,
instrumentGenericPool,
];
}
Loading