MySQL databases - service naming (#3057)

* add v0 naming to mysql integrations * add v1 to mysql integrations * add tests on v1 for mysql databases --------- Co-authored-by: Thomas Hunter II <tlhunter@datadog.com>
DataDog · Jun 20, 2023 · b06369f · b06369f
1 parent 405d58f
commit b06369f
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 21 deletions.
diff --git a/packages/datadog-plugin-mariadb/test/index.spec.js b/packages/datadog-plugin-mariadb/test/index.spec.js
@@ -5,6 +5,8 @@ const agent = require('../../dd-trace/test/plugins/agent')
 const proxyquire = require('proxyquire').noPreserveCache()
 const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants')
 
+const namingSchema = require('./naming')
+
 // https://github.com/mariadb-corporation/mariadb-connector-nodejs/commit/0a90b71ab20ab4e8b6a86a77ba291bba8ba6a34e
 const range = semver.gte(process.version, '15.0.0') ? '>=2.5.1' : '>=2'
 
@@ -86,7 +88,8 @@ describe('Plugin', () => {
         it('should do automatic instrumentation', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mariadb')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -108,7 +111,8 @@ describe('Plugin', () => {
           it('should support prepared statement shorthand', done => {
             agent
               .use(traces => {
-                expect(traces[0][0]).to.have.property('service', 'test-mariadb')
+                expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+                expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
                 expect(traces[0][0]).to.have.property('resource', 'SELECT ? + ? AS solution')
                 expect(traces[0][0]).to.have.property('type', 'sql')
                 expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -129,7 +133,8 @@ describe('Plugin', () => {
           it('should support prepared statements', done => {
             agent
               .use(traces => {
-                expect(traces[0][0]).to.have.property('service', 'test-mariadb')
+                expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+                expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
                 expect(traces[0][0]).to.have.property('resource', 'SELECT ? + ? AS solution')
                 expect(traces[0][0]).to.have.property('type', 'sql')
                 expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -293,7 +298,8 @@ describe('Plugin', () => {
         it('should do automatic instrumentation', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mariadb')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')

diff --git a/packages/datadog-plugin-mariadb/test/naming.js b/packages/datadog-plugin-mariadb/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'mariadb.query',
+      serviceName: 'test-mariadb'
+    },
+    v1: {
+      opName: 'mariadb.query',
+      serviceName: 'test'
+    }
+  }
+})
diff --git a/packages/datadog-plugin-mysql/src/index.js b/packages/datadog-plugin-mysql/src/index.js
@@ -8,9 +8,8 @@ class MySQLPlugin extends DatabasePlugin {
   static get system () { return 'mysql' }
 
   start (payload) {
-    const service = getServiceName(this.config, payload.conf)
-
-    this.startSpan(`${this.system}.query`, {
+    const service = this.serviceName(this.config, payload.conf, this.system)
+    this.startSpan(this.operationName(), {
       service,
       resource: payload.sql,
       type: 'sql',
@@ -27,12 +26,4 @@ class MySQLPlugin extends DatabasePlugin {
   }
 }
 
-function getServiceName (config, dbConfig) {
-  if (typeof config.service === 'function') {
-    return config.service(dbConfig)
-  }
-
-  return config.service
-}
-
 module.exports = MySQLPlugin
diff --git a/packages/datadog-plugin-mysql/test/index.spec.js b/packages/datadog-plugin-mysql/test/index.spec.js
@@ -4,6 +4,8 @@ const agent = require('../../dd-trace/test/plugins/agent')
 const proxyquire = require('proxyquire').noPreserveCache()
 const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants')
 
+const namingSchema = require('./naming')
+
 describe('Plugin', () => {
   let mysql
   let tracer
@@ -67,7 +69,8 @@ describe('Plugin', () => {
 
         it('should do automatic instrumentation', done => {
           agent.use(traces => {
-            expect(traces[0][0]).to.have.property('service', 'test-mysql')
+            expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+            expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
             expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
             expect(traces[0][0]).to.have.property('type', 'sql')
             expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -134,6 +137,7 @@ describe('Plugin', () => {
 
         it('should be configured with the correct values', done => {
           agent.use(traces => {
+            expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
             expect(traces[0][0]).to.have.property('service', 'custom')
             done()
           })
@@ -167,6 +171,7 @@ describe('Plugin', () => {
 
         it('should be configured with the correct values', done => {
           agent.use(traces => {
+            expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
             expect(traces[0][0]).to.have.property('service', 'custom')
             sinon.assert.calledWith(serviceSpy, sinon.match({
               host: 'localhost',
@@ -203,7 +208,8 @@ describe('Plugin', () => {
 
         it('should do automatic instrumentation', done => {
           agent.use(traces => {
-            expect(traces[0][0]).to.have.property('service', 'test-mysql')
+            expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+            expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
             expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
             expect(traces[0][0]).to.have.property('type', 'sql')
             expect(traces[0][0].meta).to.have.property('span.kind', 'client')

diff --git a/packages/datadog-plugin-mysql/test/naming.js b/packages/datadog-plugin-mysql/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'mysql.query',
+      serviceName: 'test-mysql'
+    },
+    v1: {
+      opName: 'mysql.query',
+      serviceName: 'test'
+    }
+  }
+})
diff --git a/packages/datadog-plugin-mysql2/test/index.spec.js b/packages/datadog-plugin-mysql2/test/index.spec.js
@@ -4,6 +4,8 @@ const agent = require('../../dd-trace/test/plugins/agent')
 const proxyquire = require('proxyquire').noPreserveCache()
 const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants')
 
+const namingSchema = require('./naming')
+
 describe('Plugin', () => {
   let mysql2
   let tracer
@@ -74,7 +76,8 @@ describe('Plugin', () => {
         it('should do automatic instrumentation', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mysql')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -94,7 +97,8 @@ describe('Plugin', () => {
         it('should support prepared statement shorthand', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mysql')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT ? + ? AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -116,7 +120,8 @@ describe('Plugin', () => {
         it('should support prepared statements', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mysql')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT ? + ? AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -262,7 +267,8 @@ describe('Plugin', () => {
         it('should do automatic instrumentation', done => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('service', 'test-mysql')
+              expect(traces[0][0]).to.have.property('name', namingSchema.outbound.opName)
+              expect(traces[0][0]).to.have.property('service', namingSchema.outbound.serviceName)
               expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution')
               expect(traces[0][0]).to.have.property('type', 'sql')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')

diff --git a/packages/datadog-plugin-mysql2/test/naming.js b/packages/datadog-plugin-mysql2/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'mysql.query',
+      serviceName: 'test-mysql'
+    },
+    v1: {
+      opName: 'mysql.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
@@ -12,6 +12,13 @@ function fromSystem (service, system) {
   return system ? `${service}-${system}` : undefined
 }
 
+function mysqlServiceName (service, config, dbConfig, system) {
+  if (typeof config.service === 'function') {
+    return config.service(dbConfig)
+  }
+  return config.service ? config.service : fromSystem(service, system)
+}
+
 const redisConfig = {
   opName: () => 'redis.command',
   serviceName: (service, config, system, connectionName) => {
@@ -22,10 +29,22 @@ const redisConfig = {
 const storage = {
   client: {
     ioredis: redisConfig,
+    mariadb: {
+      opName: () => 'mariadb.query',
+      serviceName: mysqlServiceName
+    },
     memcached: {
       opName: () => 'memcached.command',
       serviceName: (service, config, system) => config.service || fromSystem(service, system)
     },
+    mysql: {
+      opName: () => 'mysql.query',
+      serviceName: mysqlServiceName
+    },
+    mysql2: {
+      opName: () => 'mysql.query',
+      serviceName: mysqlServiceName
+    },
     redis: redisConfig,
     tedious: {
       opName: () => 'tedious.request',

diff --git a/packages/dd-trace/src/service-naming/schemas/v1/storage.js b/packages/dd-trace/src/service-naming/schemas/v1/storage.js
@@ -1,3 +1,5 @@
+const { identityService } = require('../util')
+
 function configWithFallback (service, config) {
   return config.service || service
 }
@@ -7,13 +9,24 @@ const redisNaming = {
   serviceName: configWithFallback
 }
 
+const mySQLNaming = {
+  opName: () => 'mysql.query',
+  serviceName: identityService
+}
+
 const storage = {
   client: {
     ioredis: redisNaming,
+    mariadb: {
+      opName: () => 'mariadb.query',
+      serviceName: identityService
+    },
     memcached: {
       opName: () => 'memcached.command',
       serviceName: configWithFallback
     },
+    mysql: mySQLNaming,
+    mysql2: mySQLNaming,
     redis: redisNaming,
     tedious: {
       opName: () => 'mssql.query',