From 15cc208c4e0320378c5f43787ab6ffcb5b907eb6 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Mon, 3 Apr 2023 15:53:40 +0200 Subject: [PATCH 1/7] switch to folders for versions --- .../src/service-naming/schemas/util.js | 5 ++ .../dd-trace/src/service-naming/schemas/v0.js | 66 ------------------- .../src/service-naming/schemas/v0/index.js | 4 ++ .../service-naming/schemas/v0/messaging.js | 64 ++++++++++++++++++ .../dd-trace/src/service-naming/schemas/v1.js | 58 ---------------- .../src/service-naming/schemas/v1/index.js | 4 ++ .../service-naming/schemas/v1/messaging.js | 52 +++++++++++++++ 7 files changed, 129 insertions(+), 124 deletions(-) create mode 100644 packages/dd-trace/src/service-naming/schemas/util.js delete mode 100644 packages/dd-trace/src/service-naming/schemas/v0.js create mode 100644 packages/dd-trace/src/service-naming/schemas/v0/index.js create mode 100644 packages/dd-trace/src/service-naming/schemas/v0/messaging.js delete mode 100644 packages/dd-trace/src/service-naming/schemas/v1.js create mode 100644 packages/dd-trace/src/service-naming/schemas/v1/index.js create mode 100644 packages/dd-trace/src/service-naming/schemas/v1/messaging.js diff --git a/packages/dd-trace/src/service-naming/schemas/util.js b/packages/dd-trace/src/service-naming/schemas/util.js new file mode 100644 index 00000000000..dfdb1abb95e --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/util.js @@ -0,0 +1,5 @@ +function identityService (service) { + return service +} + +module.exports = { identityService } diff --git a/packages/dd-trace/src/service-naming/schemas/v0.js b/packages/dd-trace/src/service-naming/schemas/v0.js deleted file mode 100644 index aa5eb6fc147..00000000000 --- a/packages/dd-trace/src/service-naming/schemas/v0.js +++ /dev/null @@ -1,66 +0,0 @@ -const SchemaDefinition = require('./definition') - -function amqpServiceName (service) { - return `${service}-amqp` -} - -const schema = { - messaging: { - outbound: { - amqplib: { - opName: () => 'amqp.command', - serviceName: amqpServiceName - }, - amqp10: { - opName: () => 'amqp.send', - serviceName: amqpServiceName - }, - 'google-cloud-pubsub': { - opName: () => 'pubsub.request', - serviceName: service => `${service}-pubsub` - }, - kafkajs: { - opName: () => 'kafka.produce', - serviceName: service => `${service}-kafka` - }, - rhea: { - opName: () => 'amqp.send', - serviceName: service => `${service}-amqp-producer` - } - }, - inbound: { - amqplib: { - opName: () => 'amqp.command', - serviceName: amqpServiceName - }, - amqp10: { - opName: () => 'amqp.receive', - serviceName: amqpServiceName - }, - 'google-cloud-pubsub': { - opName: () => 'pubsub.receive', - serviceName: service => service - }, - kafkajs: { - opName: () => 'kafka.consume', - serviceName: service => `${service}-kafka` - }, - rhea: { - opName: () => 'amqp.receive', - serviceName: service => service - } - }, - controlPlane: { - amqplib: { - opName: () => 'amqp.command', - serviceName: amqpServiceName - }, - 'google-cloud-pubsub': { - opName: () => 'pubsub.request', - serviceName: service => `${service}-pubsub` - } - } - } -} - -module.exports = new SchemaDefinition(schema) diff --git a/packages/dd-trace/src/service-naming/schemas/v0/index.js b/packages/dd-trace/src/service-naming/schemas/v0/index.js new file mode 100644 index 00000000000..78f4a82737b --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v0/index.js @@ -0,0 +1,4 @@ +const SchemaDefinition = require('../definition') +const messaging = require('./messaging') + +module.exports = new SchemaDefinition({ messaging }) diff --git a/packages/dd-trace/src/service-naming/schemas/v0/messaging.js b/packages/dd-trace/src/service-naming/schemas/v0/messaging.js new file mode 100644 index 00000000000..f788493db5b --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v0/messaging.js @@ -0,0 +1,64 @@ +const { identityService } = require('../util') + +function amqpServiceName (service) { + return `${service}-amqp` +} + +const messaging = { + outbound: { + amqplib: { + opName: () => 'amqp.command', + serviceName: amqpServiceName + }, + amqp10: { + opName: () => 'amqp.send', + serviceName: amqpServiceName + }, + 'google-cloud-pubsub': { + opName: () => 'pubsub.request', + serviceName: service => `${service}-pubsub` + }, + kafkajs: { + opName: () => 'kafka.produce', + serviceName: service => `${service}-kafka` + }, + rhea: { + opName: () => 'amqp.send', + serviceName: service => `${service}-amqp-producer` + } + }, + inbound: { + amqplib: { + opName: () => 'amqp.command', + serviceName: amqpServiceName + }, + amqp10: { + opName: () => 'amqp.receive', + serviceName: amqpServiceName + }, + 'google-cloud-pubsub': { + opName: () => 'pubsub.receive', + serviceName: identityService + }, + kafkajs: { + opName: () => 'kafka.consume', + serviceName: service => `${service}-kafka` + }, + rhea: { + opName: () => 'amqp.receive', + serviceName: identityService + } + }, + controlPlane: { + amqplib: { + opName: () => 'amqp.command', + serviceName: amqpServiceName + }, + 'google-cloud-pubsub': { + opName: () => 'pubsub.request', + serviceName: service => `${service}-pubsub` + } + } +} + +module.exports = messaging diff --git a/packages/dd-trace/src/service-naming/schemas/v1.js b/packages/dd-trace/src/service-naming/schemas/v1.js deleted file mode 100644 index 709e16aeb0f..00000000000 --- a/packages/dd-trace/src/service-naming/schemas/v1.js +++ /dev/null @@ -1,58 +0,0 @@ -const SchemaDefinition = require('./definition') - -function identityService (service) { - return service -} - -const amqpInbound = { - opName: () => 'amqp.process', - serviceName: identityService -} - -const amqpOutbound = { - opName: () => 'amqp.send', - serviceName: identityService -} - -const schema = { - messaging: { - outbound: { - amqplib: amqpOutbound, - amqp10: amqpOutbound, - 'google-cloud-pubsub': { - opName: () => 'gcp.pubsub.send', - serviceName: identityService - }, - kafkajs: { - opName: () => 'kafka.send', - serviceName: identityService - }, - rhea: amqpOutbound - }, - inbound: { - amqplib: amqpInbound, - amqp10: amqpInbound, - 'google-cloud-pubsub': { - opName: () => 'gcp.pubsub.process', - serviceName: identityService - }, - kafkajs: { - opName: () => 'kafka.process', - serviceName: identityService - }, - rhea: amqpInbound - }, - controlPlane: { - amqplib: { - opName: () => 'amqp.command', - serviceName: identityService - }, - 'google-cloud-pubsub': { - opName: () => 'gcp.pubsub.request', - serviceName: identityService - } - } - } -} - -module.exports = new SchemaDefinition(schema) diff --git a/packages/dd-trace/src/service-naming/schemas/v1/index.js b/packages/dd-trace/src/service-naming/schemas/v1/index.js new file mode 100644 index 00000000000..78f4a82737b --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v1/index.js @@ -0,0 +1,4 @@ +const SchemaDefinition = require('../definition') +const messaging = require('./messaging') + +module.exports = new SchemaDefinition({ messaging }) diff --git a/packages/dd-trace/src/service-naming/schemas/v1/messaging.js b/packages/dd-trace/src/service-naming/schemas/v1/messaging.js new file mode 100644 index 00000000000..afe3da2bb62 --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v1/messaging.js @@ -0,0 +1,52 @@ +const { identityService } = require('../util') + +const amqpInbound = { + opName: () => 'amqp.process', + serviceName: identityService +} + +const amqpOutbound = { + opName: () => 'amqp.send', + serviceName: identityService +} + +const messaging = { + outbound: { + amqplib: amqpOutbound, + amqp10: amqpOutbound, + 'google-cloud-pubsub': { + opName: () => 'gcp.pubsub.send', + serviceName: identityService + }, + kafkajs: { + opName: () => 'kafka.send', + serviceName: identityService + }, + rhea: amqpOutbound + }, + inbound: { + amqplib: amqpInbound, + amqp10: amqpInbound, + 'google-cloud-pubsub': { + opName: () => 'gcp.pubsub.process', + serviceName: identityService + }, + kafkajs: { + opName: () => 'kafka.process', + serviceName: identityService + }, + rhea: amqpInbound + }, + controlPlane: { + amqplib: { + opName: () => 'amqp.command', + serviceName: identityService + }, + 'google-cloud-pubsub': { + opName: () => 'gcp.pubsub.request', + serviceName: identityService + } + } +} + +module.exports = messaging From 10a372a274a4c03e3cca18384e57b5c73c27407f Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Thu, 27 Apr 2023 11:12:09 +0200 Subject: [PATCH 2/7] switch to kind as direct secondary key for naming schema --- packages/dd-trace/src/service-naming/index.js | 12 ++---------- .../src/service-naming/schemas/definition.js | 14 +++++++------- .../src/service-naming/schemas/v0/messaging.js | 6 +++--- .../src/service-naming/schemas/v1/messaging.js | 6 +++--- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/packages/dd-trace/src/service-naming/index.js b/packages/dd-trace/src/service-naming/index.js index c453a5e1630..c515c75ed2c 100644 --- 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 @@ -23,11 +15,11 @@ class SchemaManager { } opName (type, kind, plugin, opNameArgs) { - return this.schema.getOpName(type, kindMap[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) + 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 index 983cb363534..56a565d5bbe 100644 --- a/packages/dd-trace/src/service-naming/schemas/definition.js +++ b/packages/dd-trace/src/service-naming/schemas/definition.js @@ -3,20 +3,20 @@ 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) + 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) + 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/v0/messaging.js b/packages/dd-trace/src/service-naming/schemas/v0/messaging.js index f788493db5b..0887d1169c7 100644 --- a/packages/dd-trace/src/service-naming/schemas/v0/messaging.js +++ b/packages/dd-trace/src/service-naming/schemas/v0/messaging.js @@ -5,7 +5,7 @@ function amqpServiceName (service) { } const messaging = { - outbound: { + producer: { amqplib: { opName: () => 'amqp.command', serviceName: amqpServiceName @@ -27,7 +27,7 @@ const messaging = { serviceName: service => `${service}-amqp-producer` } }, - inbound: { + consumer: { amqplib: { opName: () => 'amqp.command', serviceName: amqpServiceName @@ -49,7 +49,7 @@ const messaging = { serviceName: identityService } }, - controlPlane: { + client: { amqplib: { opName: () => 'amqp.command', serviceName: amqpServiceName diff --git a/packages/dd-trace/src/service-naming/schemas/v1/messaging.js b/packages/dd-trace/src/service-naming/schemas/v1/messaging.js index afe3da2bb62..ee734516eb7 100644 --- a/packages/dd-trace/src/service-naming/schemas/v1/messaging.js +++ b/packages/dd-trace/src/service-naming/schemas/v1/messaging.js @@ -11,7 +11,7 @@ const amqpOutbound = { } const messaging = { - outbound: { + producer: { amqplib: amqpOutbound, amqp10: amqpOutbound, 'google-cloud-pubsub': { @@ -24,7 +24,7 @@ const messaging = { }, rhea: amqpOutbound }, - inbound: { + consumer: { amqplib: amqpInbound, amqp10: amqpInbound, 'google-cloud-pubsub': { @@ -37,7 +37,7 @@ const messaging = { }, rhea: amqpInbound }, - controlPlane: { + client: { amqplib: { opName: () => 'amqp.command', serviceName: identityService From 708b22313a2599c060ff18753184347f8b950536 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Tue, 25 Apr 2023 15:23:02 +0200 Subject: [PATCH 3/7] switch to positional arguments for extra args --- packages/dd-trace/src/plugins/tracing.js | 8 ++++---- packages/dd-trace/src/service-naming/index.js | 8 ++++---- .../dd-trace/src/service-naming/schemas/definition.js | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/dd-trace/src/plugins/tracing.js b/packages/dd-trace/src/plugins/tracing.js index 0f2ecc28547..c0ad2e3d0f5 100644 --- 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 index c515c75ed2c..5457db09d15 100644 --- a/packages/dd-trace/src/service-naming/index.js +++ b/packages/dd-trace/src/service-naming/index.js @@ -14,12 +14,12 @@ class SchemaManager { return this.config.spanAttributeSchema } - opName (type, kind, plugin, opNameArgs) { - return this.schema.getOpName(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, 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 index 56a565d5bbe..4a856d47b52 100644 --- a/packages/dd-trace/src/service-naming/schemas/definition.js +++ b/packages/dd-trace/src/service-naming/schemas/definition.js @@ -10,14 +10,14 @@ class SchemaDefinition { } } - getOpName (type, kind, plugin, opNameArgs) { + getOpName (type, kind, plugin, ...opNameArgs) { const item = this.getSchemaItem(type, kind, plugin) - return item.opName(opNameArgs) + return item.opName(...opNameArgs) } - getServiceName (type, kind, plugin, service, serviceNameArgs) { + getServiceName (type, kind, plugin, service, ...serviceNameArgs) { const item = this.getSchemaItem(type, kind, plugin) - return item.serviceName(service, serviceNameArgs) + return item.serviceName(service, ...serviceNameArgs) } } From ce382cddbce005ed55911dfab5398970378d5405 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Mon, 3 Apr 2023 19:02:10 +0200 Subject: [PATCH 4/7] add v0 to all cache integrations --- .../datadog-plugin-memcached/src/index.js | 5 +-- packages/datadog-plugin-redis/src/index.js | 14 +------ packages/dd-trace/src/plugins/cache.js | 7 ++++ packages/dd-trace/src/plugins/storage.js | 2 + .../src/service-naming/schemas/v0/index.js | 3 +- .../src/service-naming/schemas/v0/storage.js | 39 +++++++++++++++++++ 6 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 packages/dd-trace/src/service-naming/schemas/v0/storage.js diff --git a/packages/datadog-plugin-memcached/src/index.js b/packages/datadog-plugin-memcached/src/index.js index bd2b641a115..7ed447cff85 100644 --- 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-redis/src/index.js b/packages/datadog-plugin-redis/src/index.js index b7e13795b1f..03f86d54696 100644 --- a/packages/datadog-plugin-redis/src/index.js +++ b/packages/datadog-plugin-redis/src/index.js @@ -13,9 +13,9 @@ 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: { @@ -33,16 +33,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/dd-trace/src/plugins/cache.js b/packages/dd-trace/src/plugins/cache.js index 990f2d11f92..461d32ac685 100644 --- 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 index 5a4e4cc52db..8d79b4544c7 100644 --- 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/service-naming/schemas/v0/index.js b/packages/dd-trace/src/service-naming/schemas/v0/index.js index 78f4a82737b..faccf370851 100644 --- a/packages/dd-trace/src/service-naming/schemas/v0/index.js +++ b/packages/dd-trace/src/service-naming/schemas/v0/index.js @@ -1,4 +1,5 @@ const SchemaDefinition = require('../definition') const messaging = require('./messaging') +const storage = require('./storage') -module.exports = new SchemaDefinition({ messaging }) +module.exports = new SchemaDefinition({ messaging, storage }) diff --git a/packages/dd-trace/src/service-naming/schemas/v0/storage.js b/packages/dd-trace/src/service-naming/schemas/v0/storage.js new file mode 100644 index 00000000000..7e50a18ecf7 --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v0/storage.js @@ -0,0 +1,39 @@ +function getRedisService (config, connectionName) { + if (config.splitByInstance && connectionName) { + return config.service + ? `${config.service}-${connectionName}` + : connectionName + } + + return config.service +} + +function fromSystem (service, system) { + return system ? `${service}-${system}` : undefined +} + +const redisConfig = { + opName: () => 'redis.command', + serviceName: (service, config, system, connectionName) => { + return getRedisService(config, connectionName) || fromSystem(service, system) + } +} + +const storage = { + client: { + ioredis: { + opName: () => 'redis.command', + serviceName: (service, opts) => getRedisService(opts.config, opts.connectionName) + }, + memcached: { + opName: () => 'memcached.command', + serviceName: (service, config, system) => config.service || fromSystem(service, system) + }, + redis: { + opName: () => 'redis.command', + serviceName: service => `${service}-redis` + } + } +} + +module.exports = storage From c1a8c132b78bd2e96035344330409b6ca9b6fde5 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Mon, 3 Apr 2023 19:02:27 +0200 Subject: [PATCH 5/7] add v1 to all cache integrations --- .../src/service-naming/schemas/v1/index.js | 3 ++- .../src/service-naming/schemas/v1/storage.js | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 packages/dd-trace/src/service-naming/schemas/v1/storage.js diff --git a/packages/dd-trace/src/service-naming/schemas/v1/index.js b/packages/dd-trace/src/service-naming/schemas/v1/index.js index 78f4a82737b..faccf370851 100644 --- a/packages/dd-trace/src/service-naming/schemas/v1/index.js +++ b/packages/dd-trace/src/service-naming/schemas/v1/index.js @@ -1,4 +1,5 @@ const SchemaDefinition = require('../definition') const messaging = require('./messaging') +const storage = require('./storage') -module.exports = new SchemaDefinition({ messaging }) +module.exports = new SchemaDefinition({ messaging, storage }) diff --git a/packages/dd-trace/src/service-naming/schemas/v1/storage.js b/packages/dd-trace/src/service-naming/schemas/v1/storage.js new file mode 100644 index 00000000000..75dbbeef9a5 --- /dev/null +++ b/packages/dd-trace/src/service-naming/schemas/v1/storage.js @@ -0,0 +1,21 @@ +function configWithFallback (service, config) { + return config.service || service +} + +const redisNaming = { + opName: () => 'redis.command', + serviceName: configWithFallback +} + +const storage = { + client: { + ioredis: redisNaming, + memcached: { + opName: () => 'memcached.command', + serviceName: configWithFallback + }, + redis: redisNaming + } +} + +module.exports = storage From b6f537cccd7b2872792c96b24fa392af14390807 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Mon, 3 Apr 2023 19:02:58 +0200 Subject: [PATCH 6/7] add tests for all cache versions --- packages/datadog-plugin-ioredis/test/index.spec.js | 10 ++++++---- packages/datadog-plugin-ioredis/test/naming.js | 14 ++++++++++++++ .../datadog-plugin-memcached/test/index.spec.js | 6 ++++-- packages/datadog-plugin-memcached/test/naming.js | 14 ++++++++++++++ packages/datadog-plugin-redis/src/index.js | 1 - packages/datadog-plugin-redis/test/client.spec.js | 10 ++++++---- packages/datadog-plugin-redis/test/legacy.spec.js | 6 ++++-- packages/datadog-plugin-redis/test/naming.js | 14 ++++++++++++++ .../src/service-naming/schemas/v0/storage.js | 10 ++-------- 9 files changed, 64 insertions(+), 21 deletions(-) create mode 100644 packages/datadog-plugin-ioredis/test/naming.js create mode 100644 packages/datadog-plugin-memcached/test/naming.js create mode 100644 packages/datadog-plugin-redis/test/naming.js diff --git a/packages/datadog-plugin-ioredis/test/index.spec.js b/packages/datadog-plugin-ioredis/test/index.spec.js index 29bb6edcaa0..fad95b2ae48 100644 --- a/packages/datadog-plugin-ioredis/test/index.spec.js +++ b/packages/datadog-plugin-ioredis/test/index.spec.js @@ -4,6 +4,8 @@ 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 namingSchema = require('./naming') + describe('Plugin', () => { let Redis let redis @@ -30,8 +32,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 +86,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') diff --git a/packages/datadog-plugin-ioredis/test/naming.js b/packages/datadog-plugin-ioredis/test/naming.js new file mode 100644 index 00000000000..ed2ceaf8eb6 --- /dev/null +++ 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/test/index.spec.js b/packages/datadog-plugin-memcached/test/index.spec.js index ee095a16954..8cc8a7d9ef0 100644 --- 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') diff --git a/packages/datadog-plugin-memcached/test/naming.js b/packages/datadog-plugin-memcached/test/naming.js new file mode 100644 index 00000000000..6eb79f2b956 --- /dev/null +++ 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 index 03f86d54696..37149e7f847 100644 --- a/packages/datadog-plugin-redis/src/index.js +++ b/packages/datadog-plugin-redis/src/index.js @@ -17,7 +17,6 @@ class RedisPlugin extends CachePlugin { resource, service: this.serviceName(this.config, this.system, connectionName), type: 'redis', - kind: 'client', meta: { 'db.type': 'redis', 'db.name': db || '0', diff --git a/packages/datadog-plugin-redis/test/client.spec.js b/packages/datadog-plugin-redis/test/client.spec.js index cbc1f2d415f..a7404edd1ab 100644 --- 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') diff --git a/packages/datadog-plugin-redis/test/legacy.spec.js b/packages/datadog-plugin-redis/test/legacy.spec.js index c45a9249a2f..3078a5c5079 100644 --- 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') diff --git a/packages/datadog-plugin-redis/test/naming.js b/packages/datadog-plugin-redis/test/naming.js new file mode 100644 index 00000000000..ed2ceaf8eb6 --- /dev/null +++ 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/service-naming/schemas/v0/storage.js b/packages/dd-trace/src/service-naming/schemas/v0/storage.js index 7e50a18ecf7..30eecdf6e7d 100644 --- a/packages/dd-trace/src/service-naming/schemas/v0/storage.js +++ b/packages/dd-trace/src/service-naming/schemas/v0/storage.js @@ -21,18 +21,12 @@ const redisConfig = { const storage = { client: { - ioredis: { - opName: () => 'redis.command', - serviceName: (service, opts) => getRedisService(opts.config, opts.connectionName) - }, + ioredis: redisConfig, memcached: { opName: () => 'memcached.command', serviceName: (service, config, system) => config.service || fromSystem(service, system) }, - redis: { - opName: () => 'redis.command', - serviceName: service => `${service}-redis` - } + redis: redisConfig } } From ccada4953f41ff0e8c5fae1ae65a9b5471da8093 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Mon, 24 Apr 2023 10:42:14 +0200 Subject: [PATCH 7/7] add v1 tests to cache integrations --- .../datadog-plugin-ioredis/test/index.spec.js | 16 ++++++++++++++++ .../datadog-plugin-memcached/test/index.spec.js | 6 ++++++ .../datadog-plugin-redis/test/client.spec.js | 12 ++++++++++++ .../datadog-plugin-redis/test/legacy.spec.js | 12 ++++++++++++ 4 files changed, 46 insertions(+) diff --git a/packages/datadog-plugin-ioredis/test/index.spec.js b/packages/datadog-plugin-ioredis/test/index.spec.js index fad95b2ae48..7e0bedeb76e 100644 --- a/packages/datadog-plugin-ioredis/test/index.spec.js +++ b/packages/datadog-plugin-ioredis/test/index.spec.js @@ -4,6 +4,7 @@ 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', () => { @@ -105,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', () => { @@ -137,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-memcached/test/index.spec.js b/packages/datadog-plugin-memcached/test/index.spec.js index 8cc8a7d9ef0..5bd6029ad18 100644 --- a/packages/datadog-plugin-memcached/test/index.spec.js +++ b/packages/datadog-plugin-memcached/test/index.spec.js @@ -143,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-redis/test/client.spec.js b/packages/datadog-plugin-redis/test/client.spec.js index a7404edd1ab..5a15c2b11d7 100644 --- a/packages/datadog-plugin-redis/test/client.spec.js +++ b/packages/datadog-plugin-redis/test/client.spec.js @@ -94,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', () => { @@ -136,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 index 3078a5c5079..b6c8da073ba 100644 --- a/packages/datadog-plugin-redis/test/legacy.spec.js +++ b/packages/datadog-plugin-redis/test/legacy.spec.js @@ -137,6 +137,12 @@ describe('Legacy Plugin', () => { assertError() }) }) + + withNamingSchema( + () => client.get('foo', () => {}), + () => namingSchema.outbound.opName, + () => namingSchema.outbound.serviceName + ) }) describe('with configuration', () => { @@ -179,6 +185,12 @@ describe('Legacy Plugin', () => { client.get('foo', () => {}) }) + + withNamingSchema( + () => client.get('foo', () => {}), + () => namingSchema.outbound.opName, + () => 'custom' + ) }) describe('with legacy configuration', () => {