Tedious - service naming (#3061)

* add v0 naming to tedious (mssql) * add v1 naming to tedious (mssql) * 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] microsoft/mssql-docker#668 [2] https://hub.docker.com/_/microsoft-azure-sql-edge [3] https://learn.microsoft.com/en-us/azure/azure-sql-edge/features#unsupported-features
DataDog · Jun 23, 2023 · 52f4fd7 · 52f4fd7
1 parent 377114b
commit 52f4fd7
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 25 deletions.
diff --git 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"

diff --git 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
@@ -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)
@@ -123,8 +136,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 +161,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 +180,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 +197,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 +215,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 +236,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 +257,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 +348,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 +364,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
@@ -1,13 +1,13 @@
-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',
+      opName: 'mssql.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
@@ -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)
+    }
   }
 }
 

diff --git 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
+    }
   }
 }