From 45432fa186c9cd9e7dd6fb335db77998d70f0a5a Mon Sep 17 00:00:00 2001 From: Chris Raible Date: Wed, 5 Feb 2025 18:47:36 -0800 Subject: [PATCH] Removed connection pool instrumentation (#22091) no issue - This instrumentation is not currently being used and we have a safer method to get this data from the connection pool using prometheus. --- .../data/db/ConnectionPoolInstrumentation.js | 142 ----------- ghost/core/core/server/data/db/connection.js | 6 - .../data/db/InstrumentConnectionPool.test.js | 228 ------------------ 3 files changed, 376 deletions(-) delete mode 100644 ghost/core/core/server/data/db/ConnectionPoolInstrumentation.js delete mode 100644 ghost/core/test/unit/server/data/db/InstrumentConnectionPool.test.js diff --git a/ghost/core/core/server/data/db/ConnectionPoolInstrumentation.js b/ghost/core/core/server/data/db/ConnectionPoolInstrumentation.js deleted file mode 100644 index 2df9f47613..0000000000 --- a/ghost/core/core/server/data/db/ConnectionPoolInstrumentation.js +++ /dev/null @@ -1,142 +0,0 @@ -class ConnectionPoolInstrumentation { - constructor({knex, logging, metrics, config}) { - this.knex = knex; - this.pool = this.knex.client.pool; - this.logging = logging; - this.metrics = metrics; - this.enabled = config.get('telemetry:connectionPool'); - this.requestStartTimes = {}; - this.createStartTimes = {}; - } - - // Get the number of pending creates and acquires plus free and used connections - getPoolMetrics() { - return { - numPendingCreates: this.pool.numPendingCreates(), - numPendingAcquires: this.pool.numPendingAcquires(), - numFree: this.pool.numFree(), - numUsed: this.pool.numUsed() - }; - } - - handleCreateRequest(eventId) { - // Track when the request was submitted for calculating wait time - this.createStartTimes[eventId] = Date.now(); - const poolMetrics = this.getPoolMetrics(); - this.logging.debug(`[ConnectionPool] Creating a connection. EventID: ${eventId} Pending Creates: ${poolMetrics.numPendingCreates} Free: ${poolMetrics.numFree} Used: ${poolMetrics.numUsed}`); - this.metrics.metric('connection-pool', { - event: 'create-request', - eventId, - ...poolMetrics - }); - } - - handleCreateSuccess(eventId, resource) { - // Calculate the time it took to create the connection - const timeToCreate = Date.now() - this.createStartTimes[eventId]; - // Delete the start time so we don't leak memory - delete this.createStartTimes[eventId]; - this.logging.debug(`[ConnectionPool] Created a connection. EventID: ${eventId} Connection ID: ${resource.connectionId} Knex ID: ${resource.__knexUid} Time to Create: ${timeToCreate}ms`); - this.metrics.metric('connection-pool', { - event: 'create-success', - eventId, - connectionId: resource.connectionId, - knexUid: resource.__knexUid, - timeToCreate - }); - } - - handleCreateFail(eventId, err) { - // Calculate the time it took to create the connection - const timeToFail = Date.now() - this.createStartTimes[eventId]; - // Delete the start time so we don't leak memory - delete this.createStartTimes[eventId]; - const poolMetrics = this.getPoolMetrics(); - this.logging.error(`[ConnectionPool] Failed to create a connection. EventID: ${eventId} Time to Create: ${timeToFail}ms`, err); - this.metrics.metric('connection-pool', { - event: 'create-fail', - eventId, - timeToFail, - ...poolMetrics - }); - } - - handleAcquireRequest(eventId) { - // Track when the request was submitted for calculating wait time - this.requestStartTimes[eventId] = Date.now(); - const poolMetrics = this.getPoolMetrics(); - this.logging.debug(`[ConnectionPool] Acquiring a connection. EventID: ${eventId} Pending Acquires: ${poolMetrics.numPendingAcquires} Free: ${poolMetrics.numFree} Used: ${poolMetrics.numUsed}`); - this.metrics.metric('connection-pool', { - event: 'acquire-request', - eventId, - ...poolMetrics - }); - } - - handleAcquireSuccess(eventId, resource) { - // Calculate the time it took to acquire the connection - const timeToAcquire = Date.now() - this.requestStartTimes[eventId]; - // Delete the start time so we don't leak memory - delete this.requestStartTimes[eventId]; - // Track when the connection was acquired for calculating lifetime upon release - resource.connectionAcquired = Date.now(); - this.logging.debug(`[ConnectionPool] Acquired a connection. EventID: ${eventId} Connection ID: ${resource.connectionId} Knex Id: ${resource.__knexUid} Time to Acquire: ${timeToAcquire}ms`); - this.metrics.metric('connection-pool', { - event: 'acquire-success', - eventId, - connectionId: resource.connectionId, - knexUid: resource.__knexUid, - timeToAcquire - }); - } - - handleAcquireFail(eventId, err) { - // Calculate the time it took to acquire the connection - const timeToFail = Date.now() - this.requestStartTimes[eventId]; - // Delete the start time so we don't leak memory - delete this.requestStartTimes[eventId]; - const poolMetrics = this.getPoolMetrics(); - this.logging.error(`[ConnectionPool] Failed to acquire a connection. EventID: ${eventId} Time to Acquire: ${timeToFail}ms`, err); - this.metrics.metric('connection-pool', { - event: 'acquire-fail', - eventId, - timeToFail, - ...poolMetrics - }); - } - - handleRelease(resource) { - const lifetime = Date.now() - resource.connectionAcquired; - this.logging.debug(`[ConnectionPool] Released a connection. Connection ID: ${resource.connectionId} Lifetime: ${lifetime}ms`); - this.metrics.metric('connection-pool', { - event: 'release', - connectionId: resource.connectionId, - knexUid: resource.__knexUid, - lifetime - }); - } - - instrument() { - if (this.enabled) { - // Check to make sure these event listeners haven't already been added - if (this.pool.emitter.eventNames().length === 0) { - // Fires when a connection is requested to be created - this.pool.on('createRequest', eventId => this.handleCreateRequest(eventId)); - // Fires when a connection is successfully created - this.pool.on('createSuccess', (eventId, resource) => this.handleCreateSuccess(eventId, resource)); - // Fires when a connection fails to be created - this.pool.on('createFail', (eventId, err) => this.handleCreateFail(eventId, err)); - // Fires when a connection is requested from the pool - this.pool.on('acquireRequest', eventId => this.handleAcquireRequest(eventId)); - // Fires when a connection is allocated from the pool - this.pool.on('acquireSuccess', (eventId, resource) => this.handleAcquireSuccess(eventId, resource)); - // Fires when a connection fails to be allocated from the pool - this.pool.on('acquireFail', (eventId, err) => this.handleAcquireFail(eventId, err)); - // Fires when a connection is released back into the pool - this.pool.on('release', resource => this.handleRelease(resource)); - } - } - } -} - -module.exports = ConnectionPoolInstrumentation; diff --git a/ghost/core/core/server/data/db/connection.js b/ghost/core/core/server/data/db/connection.js index 4cb555de50..2786521531 100644 --- a/ghost/core/core/server/data/db/connection.js +++ b/ghost/core/core/server/data/db/connection.js @@ -4,10 +4,8 @@ const os = require('os'); const fs = require('fs'); const logging = require('@tryghost/logging'); -const metrics = require('@tryghost/metrics'); const config = require('../../../shared/config'); const errors = require('@tryghost/errors'); -const ConnectionPoolInstrumentation = require('./ConnectionPoolInstrumentation'); let knexInstance; // @TODO: @@ -64,10 +62,6 @@ function configure(dbConfig) { if (!knexInstance && config.get('database') && config.get('database').client) { knexInstance = knex(configure(config.get('database'))); - if (config.get('telemetry:connectionPool')) { - const instrumentation = new ConnectionPoolInstrumentation({knex: knexInstance, logging, metrics, config}); - instrumentation.instrument(); - } } module.exports = knexInstance; diff --git a/ghost/core/test/unit/server/data/db/InstrumentConnectionPool.test.js b/ghost/core/test/unit/server/data/db/InstrumentConnectionPool.test.js deleted file mode 100644 index 0f065166bb..0000000000 --- a/ghost/core/test/unit/server/data/db/InstrumentConnectionPool.test.js +++ /dev/null @@ -1,228 +0,0 @@ -const sinon = require('sinon'); -const ConnectionPoolInstrumentation = require('../../../../../core/server/data/db/ConnectionPoolInstrumentation'); -const should = require('should'); -var EventEmitter = require('events').EventEmitter; - -describe('ConnectionPoolInstrumentation', function () { - afterEach(function () { - sinon.restore(); - }); - - it('getPoolMetrics', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = {}; - const metrics = {}; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - const result = instrumentation.getPoolMetrics(); - result.should.eql({ - numPendingCreates: 1, - numPendingAcquires: 2, - numFree: 3, - numUsed: 4 - }); - }); - - it('should handle creating a request successfully', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = { - debug: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.handleCreateRequest(1); - instrumentation.handleCreateSuccess(1, {connectionId: 2, __knexUid: 3}); - logging.debug.calledTwice.should.be.true(); - metrics.metric.calledTwice.should.be.true(); - }); - - it('should handle creating a request unsuccessfully', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = { - debug: sinon.stub(), - error: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.handleCreateRequest(1); - instrumentation.handleCreateFail(1, new Error('test')); - logging.error.calledOnce.should.be.true(); - metrics.metric.calledTwice.should.be.true(); - }); - - it('should handle acquiring a connection successfully', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = { - debug: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.handleAcquireRequest(1); - instrumentation.handleAcquireSuccess(1, {connectionId: 2, __knexUid: 3}); - logging.debug.calledTwice.should.be.true(); - metrics.metric.calledTwice.should.be.true(); - }); - - it('should handle acquiring a connection unsuccessfully', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = { - debug: sinon.stub(), - error: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.handleAcquireRequest(1); - instrumentation.handleAcquireFail(1, new Error('test')); - logging.error.calledOnce.should.be.true(); - metrics.metric.calledTwice.should.be.true(); - }); - - it('should handle releasing a connection successfully', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4) - } - } - }; - const logging = { - debug: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.handleRelease(1); - logging.debug.calledOnce.should.be.true(); - metrics.metric.calledOnce.should.be.true(); - }); - - it('should instrument the connection pool if enabled', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4), - on: sinon.stub(), - emitter: new EventEmitter() - } - } - }; - const logging = { - debug: sinon.stub(), - error: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(true) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.instrument(); - knex.client.pool.on.callCount.should.eql(7); - }); - - it('should not instrument the connection pool if disabled', async function () { - const knex = { - client: { - pool: { - numPendingCreates: sinon.stub().returns(1), - numPendingAcquires: sinon.stub().returns(2), - numFree: sinon.stub().returns(3), - numUsed: sinon.stub().returns(4), - on: sinon.stub(), - emitter: new EventEmitter() - } - } - }; - const logging = { - debug: sinon.stub(), - error: sinon.stub() - }; - const metrics = { - metric: sinon.stub() - }; - const config = { - get: sinon.stub().withArgs('telemetry:connectionPool').returns(false) - }; - const instrumentation = new ConnectionPoolInstrumentation({knex, logging, metrics, config}); - instrumentation.instrument(); - knex.client.pool.on.callCount.should.eql(0); - }); -}); \ No newline at end of file