diff --git a/packages/collector/test/integration/currencies/databases/pg/app.js b/packages/collector/test/integration/currencies/databases/pg/app.js index 2843101905..796e23dd2e 100644 --- a/packages/collector/test/integration/currencies/databases/pg/app.js +++ b/packages/collector/test/integration/currencies/databases/pg/app.js @@ -49,6 +49,22 @@ pool.query(createTableQuery, err => { } }); +// Create a stored procedure for testing +const createProcedureQuery = ` + CREATE OR REPLACE FUNCTION get_user_by_name(user_name VARCHAR) + RETURNS TABLE(id INT, name VARCHAR, email VARCHAR) AS $$ + BEGIN + RETURN QUERY SELECT users.id, users.name, users.email FROM users WHERE users.name = user_name; + END; + $$ LANGUAGE plpgsql; +`; + +pool.query(createProcedureQuery, err => { + if (err) { + log('Failed to create stored procedure', err); + } +}); + if (process.env.WITH_STDOUT) { app.use(morgan(`${logPrefix}:method :url :status`)); } @@ -105,6 +121,32 @@ app.get('/parameterized-query', async (req, res) => { res.json({}); }); +app.get('/bind-variables-test', async (req, res) => { + // Test with string query and positional array parameters + await client.query('SELECT * FROM users WHERE name = $1 AND email = $2', ['testuser', 'test@example.com']); + + // Test with config object containing values + await pool.query({ + text: 'INSERT INTO users(name, email) VALUES($1, $2) RETURNING *', + values: ['bindtest', 'bindtest@example.com'] + }); + + res.json({ success: true }); +}); + +app.get('/stored-procedure-test', async (req, res) => { + // First insert a test user + await client.query('INSERT INTO users(name, email) VALUES($1, $2) ON CONFLICT DO NOTHING', [ + 'proceduretest', + 'procedure@example.com' + ]); + + // Call stored procedure with bind variable + const result = await client.query('SELECT * FROM get_user_by_name($1)', ['proceduretest']); + + res.json({ success: true, rows: result.rows }); +}); + app.get('/pool-string-insert', (req, res) => { const insert = 'INSERT INTO users(name, email) VALUES($1, $2) RETURNING *'; const values = ['beaker', 'beaker@muppets.com']; diff --git a/packages/collector/test/integration/currencies/databases/pg/app.mjs b/packages/collector/test/integration/currencies/databases/pg/app.mjs index 774d060dec..c6556b50f8 100644 --- a/packages/collector/test/integration/currencies/databases/pg/app.mjs +++ b/packages/collector/test/integration/currencies/databases/pg/app.mjs @@ -104,6 +104,32 @@ app.get('/parameterized-query', async (req, res) => { res.json({}); }); +app.get('/bind-variables-test', async (req, res) => { + // Test with string query and positional array parameters + await client.query('SELECT * FROM users WHERE name = $1 AND email = $2', ['testuser', 'test@example.com']); + + // Test with config object containing values + await pool.query({ + text: 'INSERT INTO users(name, email) VALUES($1, $2) RETURNING *', + values: ['bindtest', 'bindtest@example.com'] + }); + + res.json({ success: true }); +}); + +app.get('/stored-procedure-test', async (req, res) => { + // First insert a test user + await client.query('INSERT INTO users(name, email) VALUES($1, $2) ON CONFLICT DO NOTHING', [ + 'proceduretest', + 'procedure@example.com' + ]); + + // Call stored procedure with bind variable + const result = await client.query('SELECT * FROM get_user_by_name($1)', ['proceduretest']); + + res.json({ success: true, rows: result.rows }); +}); + app.get('/pool-string-insert', (req, res) => { const insert = 'INSERT INTO users(name, email) VALUES($1, $2) RETURNING *'; const values = ['beaker', 'beaker@muppets.com']; diff --git a/packages/collector/test/integration/currencies/databases/pg/test_base.js b/packages/collector/test/integration/currencies/databases/pg/test_base.js index b74fe336c5..e7ed2ff5bb 100644 --- a/packages/collector/test/integration/currencies/databases/pg/test_base.js +++ b/packages/collector/test/integration/currencies/databases/pg/test_base.js @@ -65,6 +65,103 @@ module.exports = function (name, version, isLatest) { ) )); + it('must not capture bind variables by default', () => + controls + .sendRequest({ + method: 'GET', + path: '/bind-variables-test' + }) + .then(() => + retry(() => + agentControls.getSpans().then(spans => { + verifyHttpEntry(spans, '/bind-variables-test'); + const pgSpans = getSpansByName(spans, 'postgres'); + pgSpans.forEach(span => { + expect(span.data.pg.binds).to.not.exist; + }); + }) + ) + )); + + describe('with INSTANA_TRACING_BIND_VARIABLES=true', () => { + before(async () => { + await controls.stop(); + controls.env.INSTANA_TRACING_BIND_VARIABLES = 'true'; + await controls.startAndWaitForAgentConnection(5000, Date.now() + config.getTestTimeout()); + }); + + after(async () => { + await controls.stop(); + delete controls.env.INSTANA_TRACING_BIND_VARIABLES; + await controls.startAndWaitForAgentConnection(5000, Date.now() + config.getTestTimeout()); + }); + + it('must capture raw bind variables (string query + array params)', () => + controls + .sendRequest({ + method: 'GET', + path: '/bind-variables-test' + }) + .then(() => + retry(() => + agentControls.getSpans().then(spans => { + verifyHttpEntry(spans, '/bind-variables-test'); + + // string query with positional array params + const selectQuery = getSpansByName(spans, 'postgres').find( + span => span.data.pg.stmt === 'SELECT * FROM users WHERE name = $1 AND email = $2' + ); + expect(selectQuery).to.exist; + expect(selectQuery.data.pg.binds).to.be.an('array'); + expect(selectQuery.data.pg.binds).to.have.lengthOf(2); + expect(selectQuery.data.pg.binds[0]).to.equal('testuser'); + expect(selectQuery.data.pg.binds[1]).to.equal('test@example.com'); + + // config object with values property + const insertQuery = getSpansByName(spans, 'postgres').find( + span => span.data.pg.stmt === 'INSERT INTO users(name, email) VALUES($1, $2) RETURNING *' + ); + expect(insertQuery).to.exist; + expect(insertQuery.data.pg.binds).to.be.an('array'); + expect(insertQuery.data.pg.binds).to.have.lengthOf(2); + expect(insertQuery.data.pg.binds[0]).to.equal('bindtest'); + expect(insertQuery.data.pg.binds[1]).to.equal('bindtest@example.com'); + }) + ) + )); + + it('must capture raw bind variables when calling stored procedures', () => + controls + .sendRequest({ + method: 'GET', + path: '/stored-procedure-test' + }) + .then(() => + retry(() => + agentControls.getSpans().then(spans => { + verifyHttpEntry(spans, '/stored-procedure-test'); + + const insertQuery = getSpansByName(spans, 'postgres').find( + span => span.data.pg.stmt && span.data.pg.stmt.includes('INSERT INTO users(name, email) VALUES($1, $2)') + ); + expect(insertQuery).to.exist; + expect(insertQuery.data.pg.binds).to.be.an('array'); + expect(insertQuery.data.pg.binds).to.have.lengthOf(2); + expect(insertQuery.data.pg.binds[0]).to.equal('proceduretest'); + expect(insertQuery.data.pg.binds[1]).to.equal('procedure@example.com'); + + const procedureCall = getSpansByName(spans, 'postgres').find( + span => span.data.pg.stmt === 'SELECT * FROM get_user_by_name($1)' + ); + expect(procedureCall).to.exist; + expect(procedureCall.data.pg.binds).to.be.an('array'); + expect(procedureCall.data.pg.binds).to.have.lengthOf(1); + expect(procedureCall.data.pg.binds[0]).to.equal('proceduretest'); + }) + ) + )); + }); + it('must trace pooled select now', () => controls .sendRequest({ diff --git a/packages/core/src/config/index.js b/packages/core/src/config/index.js index 3b862be2ef..9db1fdea2a 100644 --- a/packages/core/src/config/index.js +++ b/packages/core/src/config/index.js @@ -73,6 +73,7 @@ let currentConfig; * @property {boolean} [ignoreEndpointsDisableSuppression] * @property {boolean} [disableEOLEvents] * @property {globalStackTraceConfig} [global] + * @property {boolean} [captureBindVariables] */ /** @@ -160,7 +161,8 @@ let defaults = { }, ignoreEndpoints: {}, ignoreEndpointsDisableSuppression: false, - disableEOLEvents: false + disableEOLEvents: false, + captureBindVariables: false }, preloadOpentelemetry: false, secrets: { @@ -341,6 +343,7 @@ function normalizeTracingConfig({ userConfig = {}, defaultConfig = {}, finalConf normalizeIgnoreEndpoints({ userConfig, defaultConfig, finalConfig }); normalizeIgnoreEndpointsDisableSuppression({ userConfig, defaultConfig, finalConfig }); normalizeDisableEOLEvents({ userConfig, defaultConfig, finalConfig }); + normalizeCaptureBindVariables({ userConfig, defaultConfig, finalConfig }); } /** @@ -1074,6 +1077,29 @@ function normalizeDisableEOLEvents({ userConfig = {}, defaultConfig = {}, finalC }); } +/** + * @param {{ userConfig?: InstanaConfig|null, defaultConfig?: InstanaConfig, finalConfig?: InstanaConfig }} [options] + */ +function normalizeCaptureBindVariables({ userConfig = {}, defaultConfig = {}, finalConfig = {} } = {}) { + const { value, source } = util.resolve( + { + envValue: 'INSTANA_TRACING_BIND_VARIABLES', + inCodeValue: userConfig.tracing.captureBindVariables, + defaultValue: defaultConfig.tracing.captureBindVariables + }, + [validate.booleanValidator] + ); + + configStore.set('config.tracing.captureBindVariables', { source }); + finalConfig.tracing.captureBindVariables = value; + util.log({ + configPath: 'config.tracing.captureBindVariables', + source, + value, + envVarName: 'INSTANA_TRACING_BIND_VARIABLES' + }); +} + /** * @param {{ userConfig?: InstanaConfig|null, defaultConfig?: InstanaConfig, finalConfig?: InstanaConfig }} [options] */ diff --git a/packages/core/src/tracing/instrumentation/databases/pg.js b/packages/core/src/tracing/instrumentation/databases/pg.js index 839724eb02..8a928b0574 100644 --- a/packages/core/src/tracing/instrumentation/databases/pg.js +++ b/packages/core/src/tracing/instrumentation/databases/pg.js @@ -13,11 +13,13 @@ const constants = require('../../constants'); const cls = require('../../cls'); let isActive = false; +let captureBindVariables = false; exports.spanName = 'postgres'; exports.batchable = true; -exports.init = function init() { +exports.init = function init(config) { + captureBindVariables = config && config.tracing && config.tracing.captureBindVariables === true; hook.onModuleLoad('pg', instrumentPg); }; @@ -58,6 +60,7 @@ function instrumentedQuery(ctx, originalQuery, argsForOriginalQuery) { kind: constants.EXIT }); span.stack = tracingUtil.getStackTrace(instrumentedQuery); + span.data.pg = { stmt: tracingUtil.shortenDatabaseStatement(typeof config === 'string' ? config : config.text), host, @@ -66,6 +69,21 @@ function instrumentedQuery(ctx, originalQuery, argsForOriginalQuery) { db }; + // Capture raw bind variables if enabled + if (captureBindVariables) { + let binds; + if (typeof config === 'string') { + if (argsForOriginalQuery.length > 1 && Array.isArray(argsForOriginalQuery[1])) { + binds = argsForOriginalQuery[1]; + } + } else if (config && config.values) { + binds = config.values; + } + if (binds && binds.length > 0) { + span.data.pg.binds = binds; + } + } + let originalCallback; let callbackIndex = -1; for (let i = 1; i < argsForOriginalQuery.length; i++) { diff --git a/packages/core/test/config/normalizeConfig_test.js b/packages/core/test/config/normalizeConfig_test.js index 5e27e2805a..c985b4c622 100644 --- a/packages/core/test/config/normalizeConfig_test.js +++ b/packages/core/test/config/normalizeConfig_test.js @@ -45,6 +45,7 @@ describe('config.normalizeConfig', () => { delete process.env.INSTANA_IGNORE_ENDPOINTS; delete process.env.INSTANA_IGNORE_ENDPOINTS_PATH; delete process.env.INSTANA_IGNORE_ENDPOINTS_DISABLE_SUPPRESSION; + delete process.env.INSTANA_TRACING_BIND_VARIABLES; } describe('default configuration', () => { @@ -2094,6 +2095,63 @@ describe('config.normalizeConfig', () => { }); }); + describe('captureBindVariables configuration', () => { + it('should default captureBindVariables to false', () => { + const config = coreConfig.normalize(); + expect(config.tracing.captureBindVariables).to.equal(false); + }); + + it('should use default (false) when neither env nor config is set', () => { + const config = coreConfig.normalize({}); + expect(config.tracing.captureBindVariables).to.be.false; + }); + + it('should enable captureBindVariables via config', () => { + const config = coreConfig.normalize({ userConfig: { tracing: { captureBindVariables: true } } }); + expect(config.tracing.captureBindVariables).to.equal(true); + }); + + it('should disable captureBindVariables via config', () => { + const config = coreConfig.normalize({ userConfig: { tracing: { captureBindVariables: false } } }); + expect(config.tracing.captureBindVariables).to.equal(false); + }); + + it('should enable captureBindVariables via INSTANA_TRACING_BIND_VARIABLES=true', () => { + process.env.INSTANA_TRACING_BIND_VARIABLES = 'true'; + const config = coreConfig.normalize(); + expect(config.tracing.captureBindVariables).to.equal(true); + }); + + it('should not enable captureBindVariables when INSTANA_TRACING_BIND_VARIABLES is not "true"', () => { + process.env.INSTANA_TRACING_BIND_VARIABLES = 'false'; + const config = coreConfig.normalize(); + expect(config.tracing.captureBindVariables).to.equal(false); + }); + + it('should default to false when INSTANA_TRACING_BIND_VARIABLES is set to an invalid value', () => { + process.env.INSTANA_TRACING_BIND_VARIABLES = 'invalid'; + const config = coreConfig.normalize(); + expect(config.tracing.captureBindVariables).to.equal(false); + }); + + it('should use config value when env is not set', () => { + const config = coreConfig.normalize({ userConfig: { tracing: { captureBindVariables: true } } }); + expect(config.tracing.captureBindVariables).to.be.true; + }); + + it('should give precedence to INSTANA_TRACING_BIND_VARIABLES env var over config', () => { + process.env.INSTANA_TRACING_BIND_VARIABLES = 'true'; + const config = coreConfig.normalize({ userConfig: { tracing: { captureBindVariables: false } } }); + expect(config.tracing.captureBindVariables).to.equal(true); + }); + + it('should give precedence to INSTANA_TRACING_BIND_VARIABLES=false over config=true', () => { + process.env.INSTANA_TRACING_BIND_VARIABLES = 'false'; + const config = coreConfig.normalize({ userConfig: { tracing: { captureBindVariables: true } } }); + expect(config.tracing.captureBindVariables).to.equal(false); + }); + }); + function checkDefaults(config) { expect(config).to.be.an('object'); @@ -2124,6 +2182,7 @@ describe('config.normalizeConfig', () => { expect(config.tracing.kafka.traceCorrelation).to.be.true; expect(config.tracing.useOpentelemetry).to.equal(true); expect(config.tracing.allowRootExitSpan).to.equal(false); + expect(config.tracing.captureBindVariables).to.equal(false); expect(config.preloadOpentelemetry).to.equal(false);