From 8d9e6bf430030ff06a4e18b08101b212c84335a5 Mon Sep 17 00:00:00 2001 From: David Wittman Date: Fri, 26 Jul 2024 10:12:27 -0500 Subject: [PATCH] Prefix SQL URLs with scheme Currently the database URLs provided in SQL data are formatted like: {host}:{port}/{database} However, most (maybe all?) of the other X-Ray SDKs[1][2][3] format these URLs with the scheme prefixed in front, so: {scheme}://{host}:{port}/{database} This format is also enforced on the opentelemetry collector[4], and results in the following error when receiving traces with SQL data from this library: failed to parse out the database name in the "sql.url" field When this happens, traces are dropped by the opentelemetry collector and are not sent to AWS X-Ray. This change addresses these issues by prefixing the SQL data URLs with the scheme for both MySQL (`mysql://`) and Postgres (`postgresql://`). [1]: https://github.com/aws/aws-xray-sdk-python/blob/d3a202719e659968fe6dcc04fe14c7f3045b53e8/aws_xray_sdk/ext/sqlalchemy_core/patch.py#L30 [2]: https://github.com/aws/aws-xray-sdk-java/blob/master/aws-xray-recorder-sdk-sql-mysql/src/main/java/com/amazonaws/xray/sql/mysql/TracingInterceptor.java#L119 [3]: https://github.com/aws/aws-xray-sdk-ruby/blob/b2fce0c1f4ada747ebe16b973e1ed7069013a9a2/lib/aws-xray-sdk/facets/rails/active_record.rb#L50 [4]: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/awsxrayreceiver/internal/translator/sql.go#L38-L39 --- packages/mysql/lib/mysql_p.js | 2 +- packages/mysql/test/unit/mysql_p.test.js | 14 +++++++------- packages/postgres/lib/postgres_p.js | 2 +- packages/postgres/test/unit/postgres_p.test.js | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/mysql/lib/mysql_p.js b/packages/mysql/lib/mysql_p.js index c6aa589c..207fa7ff 100644 --- a/packages/mysql/lib/mysql_p.js +++ b/packages/mysql/lib/mysql_p.js @@ -272,7 +272,7 @@ function captureOperation(name) { function createSqlData(config, values, sql) { var commandType = values ? PREPARED : null; var data = new SqlData(DATABASE_VERS, DRIVER_VERS, config.user, - config.host + ':' + config.port + '/' + config.database, + 'mysql://' + config.host + ':' + config.port + '/' + config.database, commandType); if (process.env.AWS_XRAY_COLLECT_SQL_QUERIES && sql) { diff --git a/packages/mysql/test/unit/mysql_p.test.js b/packages/mysql/test/unit/mysql_p.test.js index e50cf634..e1f626d2 100644 --- a/packages/mysql/test/unit/mysql_p.test.js +++ b/packages/mysql/test/unit/mysql_p.test.js @@ -144,7 +144,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, config.user, - config.host + ':' + config.port + '/' + config.database, 'statement'); + 'mysql://' + config.host + ':' + config.port + '/' + config.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); @@ -256,7 +256,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(process.env.MYSQL_DATABASE_VERSION, process.env.MYSQL_DRIVER_VERSION, - conParam.user, conParam.host + ':' + conParam.port + '/' + conParam.database, 'statement'); + conParam.user, 'mysql://' + conParam.host + ':' + conParam.port + '/' + conParam.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); stubAddSql.should.have.been.calledWithExactly(sinon.match.has('sanitized_query', 'sql here')); }); @@ -269,7 +269,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(process.env.MYSQL_DATABASE_VERSION, process.env.MYSQL_DRIVER_VERSION, - conParam.user, conParam.host + ':' + conParam.port + '/' + conParam.database, 'statement'); + conParam.user, 'mysql://' + conParam.host + ':' + conParam.port + '/' + conParam.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); sinon.assert.match(sinon.match, { 'sanitized_query': undefined @@ -349,7 +349,7 @@ describe('captureMySQL', function() { resolvedConn.query('sql here').then(function() { stubDataInit.should.have.been.calledWithExactly(undefined, undefined, config.user, - config.host + ':' + config.port + '/' + config.database, 'statement'); + 'mysql://' + config.host + ':' + config.port + '/' + config.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); }); @@ -460,7 +460,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, config.user, - config.host + ':' + config.port + '/' + config.database, 'statement'); + 'mysql://' + config.host + ':' + config.port + '/' + config.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); @@ -556,7 +556,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, config.user, - config.host + ':' + config.port + '/' + config.database, 'statement'); + 'mysql://' + config.host + ':' + config.port + '/' + config.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); @@ -703,7 +703,7 @@ describe('captureMySQL', function() { query.call(connectionObj, 'sql here', [1]); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, config.user, - config.host + ':' + config.port + '/' + config.database, 'statement'); + 'mysql://' + config.host + ':' + config.port + '/' + config.database, 'statement'); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); diff --git a/packages/postgres/lib/postgres_p.js b/packages/postgres/lib/postgres_p.js index 0ab4a4fd..86d71b0f 100644 --- a/packages/postgres/lib/postgres_p.js +++ b/packages/postgres/lib/postgres_p.js @@ -126,7 +126,7 @@ function createSqlData(connParams, query) { var queryType = query.name ? PREPARED : undefined; var data = new SqlData(DATABASE_VERS, DRIVER_VERS, connParams.user, - connParams.host + ':' + connParams.port + '/' + connParams.database, + 'postgresql://' + connParams.host + ':' + connParams.port + '/' + connParams.database, queryType); if (process.env.AWS_XRAY_COLLECT_SQL_QUERIES) { data.sanitized_query = query.text; diff --git a/packages/postgres/test/unit/postgres_p.test.js b/packages/postgres/test/unit/postgres_p.test.js index 05dbf083..f0720283 100644 --- a/packages/postgres/test/unit/postgres_p.test.js +++ b/packages/postgres/test/unit/postgres_p.test.js @@ -86,7 +86,7 @@ describe('capturePostgres', function() { query.call(postgres, 'sql here'); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, conParam.user, - conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); + 'postgresql://' + conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); }); @@ -100,7 +100,7 @@ describe('capturePostgres', function() { query.call(postgres, 'sql here'); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, conParam.user, - conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); + 'postgresql://' + conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); stubAddSql.should.have.been.calledWithExactly(sinon.match.has('sanitized_query', 'sql statement here')); }); @@ -227,7 +227,7 @@ describe('capturePostgres', function() { query.call(postgres, 'sql here'); stubDataInit.should.have.been.calledWithExactly(undefined, undefined, conParam.user, - conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); + 'postgresql://' + conParam.host + ':' + conParam.port + '/' + conParam.database, undefined); stubAddSql.should.have.been.calledWithExactly(sinon.match.instanceOf(SqlData)); });