Cache integrations - service naming (#3056)

Adopt service naming schema in cache integrations --------- Co-authored-by: Thomas Hunter II <[email protected]>
DataDog · May 30, 2023 · 86ea57b · 86ea57b
1 parent 8fb22f6
commit 86ea57b
Show file tree

Hide file tree

Showing 23 changed files with 323 additions and 177 deletions.
diff --git a/packages/datadog-plugin-ioredis/test/index.spec.js b/packages/datadog-plugin-ioredis/test/index.spec.js
@@ -4,6 +4,9 @@ const agent = require('../../dd-trace/test/plugins/agent')
 const { breakThen, unbreakThen } = require('../../dd-trace/test/plugins/helpers')
 const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants')
 
+const Nomenclature = require('../../dd-trace/src/service-naming')
+const namingSchema = require('./naming')
+
 describe('Plugin', () => {
   let Redis
   let redis
@@ -30,8 +33,8 @@ describe('Plugin', () => {
           agent.use(() => {}) // wait for initial info command
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'redis.command')
-              expect(traces[0][0]).to.have.property('service', 'test-redis')
+              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', 'get')
               expect(traces[0][0]).to.have.property('type', 'redis')
               expect(traces[0][0].meta).to.have.property('component', 'ioredis')
@@ -84,8 +87,8 @@ describe('Plugin', () => {
           agent.use(() => {}) // wait for initial info command
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'redis.command')
-              expect(traces[0][0]).to.have.property('service', 'test-redis')
+              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', 'get')
               expect(traces[0][0]).to.have.property('type', 'redis')
               expect(traces[0][0].meta).to.have.property('db.name', '0')
@@ -103,6 +106,12 @@ describe('Plugin', () => {
 
           redis.get('foo').catch(done)
         })
+
+        withNamingSchema(
+          done => redis.get('foo').catch(done),
+          () => namingSchema.outbound.opName,
+          () => namingSchema.outbound.serviceName
+        )
       })
 
       describe('with configuration', () => {
@@ -135,6 +144,15 @@ describe('Plugin', () => {
 
           redis.get('foo').catch(done)
         })
+
+        withNamingSchema(
+          done => redis.get('foo').catch(done),
+          () => namingSchema.outbound.opName,
+          () => {
+            if (Nomenclature.version === 'v0') return 'custom-test'
+            return 'custom'
+          }
+        )
       })
 
       describe('with legacy configuration', () => {

diff --git a/packages/datadog-plugin-ioredis/test/naming.js b/packages/datadog-plugin-ioredis/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'redis.command',
+      serviceName: 'test-redis'
+    },
+    v1: {
+      opName: 'redis.command',
+      serviceName: 'test'
+    }
+  }
+})
diff --git a/packages/datadog-plugin-memcached/src/index.js b/packages/datadog-plugin-memcached/src/index.js
@@ -9,11 +9,10 @@ class MemcachedPlugin extends CachePlugin {
   start ({ client, server, query }) {
     const address = getAddress(client, server, query)
 
-    this.startSpan('memcached.command', {
-      service: this.config.service,
+    this.startSpan({
+      service: this.serviceName(this.config, this.system),
       resource: query.type,
       type: 'memcached',
-      kind: 'client',
       meta: {
         'memcached.command': query.command,
         'out.host': address[0],

diff --git a/packages/datadog-plugin-memcached/test/index.spec.js b/packages/datadog-plugin-memcached/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 Memcached
   let memcached
@@ -28,8 +30,8 @@ describe('Plugin', () => {
 
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'memcached.command')
-              expect(traces[0][0]).to.have.property('service', 'test-memcached')
+              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', 'get')
               expect(traces[0][0]).to.have.property('type', 'memcached')
               expect(traces[0][0].meta).to.have.property('span.kind', 'client')
@@ -141,6 +143,12 @@ describe('Plugin', () => {
             done()
           }
         })
+
+        withNamingSchema(
+          done => memcached.get('test', err => err && done(err)),
+          () => namingSchema.outbound.opName,
+          () => namingSchema.outbound.serviceName
+        )
       })
 
       describe('with configuration', () => {

diff --git a/packages/datadog-plugin-memcached/test/naming.js b/packages/datadog-plugin-memcached/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'memcached.command',
+      serviceName: 'test-memcached'
+    },
+    v1: {
+      opName: 'memcached.command',
+      serviceName: 'test'
+    }
+  }
+})
diff --git a/packages/datadog-plugin-redis/src/index.js b/packages/datadog-plugin-redis/src/index.js
@@ -13,11 +13,10 @@ class RedisPlugin extends CachePlugin {
     const normalizedCommand = command.toUpperCase()
     if (!this.config.filter(normalizedCommand)) return this.skip()
 
-    this.startSpan('redis.command', {
-      service: getService(this.config, connectionName),
+    this.startSpan({
       resource,
+      service: this.serviceName(this.config, this.system, connectionName),
       type: 'redis',
-      kind: 'client',
       meta: {
         'db.type': 'redis',
         'db.name': db || '0',
@@ -33,16 +32,6 @@ class RedisPlugin extends CachePlugin {
   }
 }
 
-function getService (config, connectionName) {
-  if (config.splitByInstance && connectionName) {
-    return config.service
-      ? `${config.service}-${connectionName}`
-      : connectionName
-  }
-
-  return config.service
-}
-
 function formatCommand (command, args) {
   if (!args || command === 'AUTH') return command
 

diff --git a/packages/datadog-plugin-redis/test/client.spec.js b/packages/datadog-plugin-redis/test/client.spec.js
@@ -5,6 +5,8 @@ const agent = require('../../dd-trace/test/plugins/agent')
 const { breakThen, unbreakThen } = require('../../dd-trace/test/plugins/helpers')
 const { ERROR_MESSAGE, ERROR_TYPE } = require('../../dd-trace/src/constants')
 
+const namingSchema = require('./naming')
+
 const modules = semver.satisfies(process.versions.node, '>=14')
   ? ['@node-redis/client', '@redis/client']
   : ['@node-redis/client']
@@ -39,8 +41,8 @@ describe('Plugin', () => {
         it('should do automatic instrumentation when using callbacks', async () => {
           const promise = agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'redis.command')
-              expect(traces[0][0]).to.have.property('service', 'test-redis')
+              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', 'GET')
               expect(traces[0][0]).to.have.property('type', 'redis')
               expect(traces[0][0].meta).to.have.property('db.name', '0')
@@ -76,8 +78,8 @@ describe('Plugin', () => {
         it('should work with userland promises', async () => {
           const promise = agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'redis.command')
-              expect(traces[0][0]).to.have.property('service', 'test-redis')
+              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', 'GET')
               expect(traces[0][0]).to.have.property('type', 'redis')
               expect(traces[0][0].meta).to.have.property('db.name', '0')
@@ -92,6 +94,12 @@ describe('Plugin', () => {
           await client.get('foo')
           await promise
         })
+
+        withNamingSchema(
+          async () => client.get('foo'),
+          () => namingSchema.outbound.opName,
+          () => namingSchema.outbound.serviceName
+        )
       })
 
       describe('with configuration', () => {
@@ -134,6 +142,12 @@ describe('Plugin', () => {
           await client.get('foo')
           await promise
         })
+
+        withNamingSchema(
+          async () => client.get('foo'),
+          () => namingSchema.outbound.opName,
+          () => 'custom'
+        )
       })
 
       describe('with blocklist', () => {

diff --git a/packages/datadog-plugin-redis/test/legacy.spec.js b/packages/datadog-plugin-redis/test/legacy.spec.js
@@ -3,6 +3,8 @@
 const agent = require('../../dd-trace/test/plugins/agent')
 const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants')
 
+const namingSchema = require('./naming')
+
 describe('Legacy Plugin', () => {
   let redis
   let tracer
@@ -42,8 +44,8 @@ describe('Legacy Plugin', () => {
           client.on('error', done)
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('name', 'redis.command')
-              expect(traces[0][0]).to.have.property('service', 'test-redis')
+              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', 'get')
               expect(traces[0][0]).to.have.property('type', 'redis')
               expect(traces[0][0].meta).to.have.property('db.name', '0')
@@ -135,6 +137,12 @@ describe('Legacy Plugin', () => {
             assertError()
           })
         })
+
+        withNamingSchema(
+          () => client.get('foo', () => {}),
+          () => namingSchema.outbound.opName,
+          () => namingSchema.outbound.serviceName
+        )
       })
 
       describe('with configuration', () => {
@@ -177,6 +185,12 @@ describe('Legacy Plugin', () => {
 
           client.get('foo', () => {})
         })
+
+        withNamingSchema(
+          () => client.get('foo', () => {}),
+          () => namingSchema.outbound.opName,
+          () => 'custom'
+        )
       })
 
       describe('with legacy configuration', () => {

diff --git a/packages/datadog-plugin-redis/test/naming.js b/packages/datadog-plugin-redis/test/naming.js
@@ -0,0 +1,14 @@
+const { resolveNaming } = require('../../dd-trace/test/plugins/helpers')
+
+module.exports = resolveNaming({
+  outbound: {
+    v0: {
+      opName: 'redis.command',
+      serviceName: 'test-redis'
+    },
+    v1: {
+      opName: 'redis.command',
+      serviceName: 'test'
+    }
+  }
+})
diff --git a/packages/dd-trace/src/plugins/cache.js b/packages/dd-trace/src/plugins/cache.js
@@ -4,6 +4,13 @@ const StoragePlugin = require('./storage')
 
 class CachePlugin extends StoragePlugin {
   static get operation () { return 'command' }
+
+  startSpan (options) {
+    if (!options.kind) {
+      options.kind = this.constructor.kind
+    }
+    return super.startSpan(this.operationName(), options)
+  }
 }
 
 module.exports = CachePlugin
diff --git a/packages/dd-trace/src/plugins/storage.js b/packages/dd-trace/src/plugins/storage.js
@@ -3,6 +3,8 @@
 const ClientPlugin = require('./client')
 
 class StoragePlugin extends ClientPlugin {
+  static get type () { return 'storage' }
+
   constructor (...args) {
     super(...args)
 

diff --git a/packages/dd-trace/src/plugins/tracing.js b/packages/dd-trace/src/plugins/tracing.js
@@ -32,14 +32,14 @@ class TracingPlugin extends Plugin {
     return store && store.span
   }
 
-  serviceName (serviceArgs) {
+  serviceName (...serviceArgs) {
     const { type, id, kind } = this.constructor
-    return Nomenclature.serviceName(type, kind, id, serviceArgs)
+    return Nomenclature.serviceName(type, kind, id, ...serviceArgs)
   }
 
-  operationName (opNameArgs) {
+  operationName (...opNameArgs) {
     const { type, id, kind } = this.constructor
-    return Nomenclature.opName(type, kind, id, opNameArgs)
+    return Nomenclature.opName(type, kind, id, ...opNameArgs)
   }
 
   configure (config) {

diff --git a/packages/dd-trace/src/service-naming/index.js b/packages/dd-trace/src/service-naming/index.js
@@ -1,13 +1,5 @@
 const { schemaDefinitions } = require('./schemas')
 
-const kindMap = {
-  messaging: {
-    client: 'controlPlane',
-    consumer: 'inbound',
-    producer: 'outbound'
-  }
-}
-
 class SchemaManager {
   constructor () {
     this.schemas = schemaDefinitions
@@ -22,12 +14,12 @@ class SchemaManager {
     return this.config.spanAttributeSchema
   }
 
-  opName (type, kind, plugin, opNameArgs) {
-    return this.schema.getOpName(type, kindMap[type][kind], plugin, opNameArgs)
+  opName (type, kind, plugin, ...opNameArgs) {
+    return this.schema.getOpName(type, kind, plugin, ...opNameArgs)
   }
 
-  serviceName (type, kind, plugin, serviceNameArgs) {
-    return this.schema.getServiceName(type, kindMap[type][kind], plugin, this.config.service, serviceNameArgs)
+  serviceName (type, kind, plugin, ...serviceNameArgs) {
+    return this.schema.getServiceName(type, kind, plugin, this.config.service, ...serviceNameArgs)
   }
 
   configure (config = {}) {

diff --git a/packages/dd-trace/src/service-naming/schemas/definition.js b/packages/dd-trace/src/service-naming/schemas/definition.js
@@ -3,21 +3,21 @@ class SchemaDefinition {
     this.schema = schema
   }
 
-  getSchemaItem (type, subType, plugin) {
+  getSchemaItem (type, kind, plugin) {
     const schema = this.schema
-    if (schema && schema[type] && schema[type][subType] && schema[type][subType][plugin]) {
-      return schema[type][subType][plugin]
+    if (schema && schema[type] && schema[type][kind] && schema[type][kind][plugin]) {
+      return schema[type][kind][plugin]
     }
   }
 
-  getOpName (type, subType, plugin, opNameArgs) {
-    const item = this.getSchemaItem(type, subType, plugin)
-    return item.opName(opNameArgs)
+  getOpName (type, kind, plugin, ...opNameArgs) {
+    const item = this.getSchemaItem(type, kind, plugin)
+    return item.opName(...opNameArgs)
   }
 
-  getServiceName (type, subType, plugin, service, serviceNameArgs) {
-    const item = this.getSchemaItem(type, subType, plugin)
-    return item.serviceName(service, serviceNameArgs)
+  getServiceName (type, kind, plugin, service, ...serviceNameArgs) {
+    const item = this.getSchemaItem(type, kind, plugin)
+    return item.serviceName(service, ...serviceNameArgs)
   }
 }
 

diff --git a/packages/dd-trace/src/service-naming/schemas/util.js b/packages/dd-trace/src/service-naming/schemas/util.js
@@ -0,0 +1,5 @@
+function identityService (service) {
+  return service
+}
+
+module.exports = { identityService }