From 81aef78cbcfe432814be626716c56465012fca6d Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Thu, 27 Apr 2023 15:49:06 +0200 Subject: [PATCH 1/3] add v0 naming to tedious (mssql) --- packages/datadog-plugin-tedious/src/index.js | 4 +-- .../datadog-plugin-tedious/test/index.spec.js | 32 +++++++++---------- .../datadog-plugin-tedious/test/naming.js | 10 ++---- .../src/service-naming/schemas/v0/storage.js | 6 +++- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/datadog-plugin-tedious/src/index.js b/packages/datadog-plugin-tedious/src/index.js index f3307ddd980..b01f498394a 100644 --- a/packages/datadog-plugin-tedious/src/index.js +++ b/packages/datadog-plugin-tedious/src/index.js @@ -9,8 +9,8 @@ class TediousPlugin extends DatabasePlugin { static get system () { return 'mssql' } start ({ queryOrProcedure, connectionConfig }) { - this.startSpan('tedious.request', { - service: this.config.service, + this.startSpan(this.operationName(), { + service: this.serviceName(this.config, this.system), resource: queryOrProcedure, type: 'sql', kind: 'client', diff --git a/packages/datadog-plugin-tedious/test/index.spec.js b/packages/datadog-plugin-tedious/test/index.spec.js index 22d6e3468e2..697ad642bc1 100644 --- a/packages/datadog-plugin-tedious/test/index.spec.js +++ b/packages/datadog-plugin-tedious/test/index.spec.js @@ -123,8 +123,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('component', 'tedious') @@ -148,8 +148,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) }) @@ -167,8 +167,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) }) @@ -184,8 +184,8 @@ describe('Plugin', () => { agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) }) .then(done) @@ -202,8 +202,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) }) @@ -223,8 +223,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', query) }) @@ -244,8 +244,8 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') - expect(traces[0][0]).to.have.property('service', 'test-mssql') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) + expect(traces[0][0]).to.have.property('service', namingSchema.client.serviceName) expect(traces[0][0]).to.have.property('resource', procedure) }) @@ -335,7 +335,7 @@ describe('Plugin', () => { agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) expect(traces[0][0]).to.have.property('resource', bulkLoad.getBulkInsertSql()) }) .then(done) @@ -351,7 +351,7 @@ describe('Plugin', () => { const promise = agent .use(traces => { - expect(traces[0][0]).to.have.property('name', 'tedious.request') + expect(traces[0][0]).to.have.property('name', namingSchema.client.opName) expect(traces[0][0]).to.have.property('resource', bulkLoad.getBulkInsertSql()) }) diff --git a/packages/datadog-plugin-tedious/test/naming.js b/packages/datadog-plugin-tedious/test/naming.js index ab3d95a4e53..d2ef1ee3e7a 100644 --- a/packages/datadog-plugin-tedious/test/naming.js +++ b/packages/datadog-plugin-tedious/test/naming.js @@ -1,14 +1,10 @@ -const { namingResolver } = require('../../dd-trace/test/plugins/helpers') +const { resolveNaming } = require('../../dd-trace/test/plugins/helpers') -module.exports = namingResolver({ - outbound: { +module.exports = resolveNaming({ + client: { v0: { opName: 'tedious.request', serviceName: 'test-mssql' - }, - v1: { - opName: 'sqlserver.query', - serviceName: 'test' } } }) diff --git a/packages/dd-trace/src/service-naming/schemas/v0/storage.js b/packages/dd-trace/src/service-naming/schemas/v0/storage.js index 30eecdf6e7d..04e716c9887 100644 --- a/packages/dd-trace/src/service-naming/schemas/v0/storage.js +++ b/packages/dd-trace/src/service-naming/schemas/v0/storage.js @@ -26,7 +26,11 @@ const storage = { opName: () => 'memcached.command', serviceName: (service, config, system) => config.service || fromSystem(service, system) }, - redis: redisConfig + redis: redisConfig, + tedious: { + opName: () => 'tedious.request', + serviceName: (service, config, system) => config.service || fromSystem(service, system) + } } } From 360be8b78d91b006e234dfe0e07d4ae1589da763 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Thu, 27 Apr 2023 16:38:47 +0200 Subject: [PATCH 2/3] add v1 naming to tedious (mssql) --- packages/datadog-plugin-tedious/test/index.spec.js | 13 +++++++++++++ packages/datadog-plugin-tedious/test/naming.js | 4 ++++ .../src/service-naming/schemas/v1/storage.js | 6 +++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/datadog-plugin-tedious/test/index.spec.js b/packages/datadog-plugin-tedious/test/index.spec.js index 697ad642bc1..fd609dd3a4a 100644 --- a/packages/datadog-plugin-tedious/test/index.spec.js +++ b/packages/datadog-plugin-tedious/test/index.spec.js @@ -3,6 +3,7 @@ const agent = require('../../dd-trace/test/plugins/agent') const semver = require('semver') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') +const namingSchema = require('./naming') const MSSQL_USERNAME = 'sa' const MSSQL_PASSWORD = 'DD_HUNTER2' @@ -71,6 +72,18 @@ describe('Plugin', () => { } }) + withNamingSchema( + done => { + const query = 'SELECT 1 + 1 AS solution' + const request = new tds.Request(query, (err) => { + if (err) return done(err) + }) + connection.execSql(request) + }, + () => namingSchema.client.opName, + () => namingSchema.client.serviceName + ) + describe('with tedious disabled', () => { beforeEach(() => { tracer.use('tedious', false) diff --git a/packages/datadog-plugin-tedious/test/naming.js b/packages/datadog-plugin-tedious/test/naming.js index d2ef1ee3e7a..be25e761e37 100644 --- a/packages/datadog-plugin-tedious/test/naming.js +++ b/packages/datadog-plugin-tedious/test/naming.js @@ -5,6 +5,10 @@ module.exports = resolveNaming({ v0: { opName: 'tedious.request', serviceName: 'test-mssql' + }, + v1: { + opName: 'mssql.query', + serviceName: 'test' } } }) diff --git a/packages/dd-trace/src/service-naming/schemas/v1/storage.js b/packages/dd-trace/src/service-naming/schemas/v1/storage.js index 75dbbeef9a5..2108fa0af9e 100644 --- a/packages/dd-trace/src/service-naming/schemas/v1/storage.js +++ b/packages/dd-trace/src/service-naming/schemas/v1/storage.js @@ -14,7 +14,11 @@ const storage = { opName: () => 'memcached.command', serviceName: configWithFallback }, - redis: redisNaming + redis: redisNaming, + tedious: { + opName: () => 'mssql.query', + serviceName: configWithFallback + } } } From 0e28f1c953817498a5d7aa68b377e4c9d9fe4876 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Thu, 27 Apr 2023 17:32:06 +0200 Subject: [PATCH 3/3] switch to a mostly working test sqlserver The standard mssql server image does not work on ARM [1]. Instead, we use `azure-sql-edge` [2], which provides a sufficient subset of mssql server API to test most of our integration. Unfortunately, this image does not support stored procedures [3], so tests related to these will still fail locally. [1] https://github.com/microsoft/mssql-docker/issues/668 [2] https://hub.docker.com/_/microsoft-azure-sql-edge [3] https://learn.microsoft.com/en-us/azure/azure-sql-edge/features#unsupported-features --- docker-compose.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index c820b2cf283..a42dcdb3a9a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,7 +12,10 @@ services: ports: - "127.0.0.1:5432:5432" mssql: - image: mcr.microsoft.com/mssql/server:2017-latest-ubuntu + # A working MSSQL server is not available on ARM. + # This image provides _most_ of sqlserver functionalities, but + # does not support stored procedures (corresponding tests will fail) + image: mcr.microsoft.com/mssql/azure-sql-edge environment: - "ACCEPT_EULA=Y" - "SA_PASSWORD=DD_HUNTER2"