Skip to content

Commit

Permalink
Prefix SQL URLs with scheme
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@dwickr
dwickr committed Jul 26, 2024
1 parent 76ce2ed commit 8d9e6bf
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/mysql/lib/mysql_p.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions packages/mysql/test/unit/mysql_p.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});

Expand Down Expand Up @@ -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'));
});
Expand All @@ -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
Expand Down Expand Up @@ -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));
});
});
Expand Down Expand Up @@ -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));
});

Expand Down Expand Up @@ -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));
});

Expand Down Expand Up @@ -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));
});

Expand Down
2 changes: 1 addition & 1 deletion packages/postgres/lib/postgres_p.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions packages/postgres/test/unit/postgres_p.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});

Expand All @@ -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'));
});
Expand Down Expand Up @@ -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));
});

Expand Down

0 comments on commit 8d9e6bf

Please sign in to comment.