instrumentation: fix service name for oracledb (#6038)

The service name should be handled by the schema, even if undefined
is passed through. Instead of falling back to the connection, use
the fallback as defined in schema v1. The service method is now
only accepting truthy strings as return value and will otherwise
fallback.

Author: BridgeAR

packages/datadog-plugin-oracledb/src/index.js

@@ -11,7 +11,7 @@ class OracledbPlugin extends DatabasePlugin {
   static get peerServicePrecursors () { return ['db.instance', 'db.hostname'] }
 
   start ({ query, connAttrs, port, hostname, dbInstance }) {
-    let service = this.serviceName({ pluginConfig: this.config, params: connAttrs })
+    const service = this.serviceName({ pluginConfig: this.config, params: connAttrs })
 
     if (hostname === undefined) {
       // Lazy load for performance. This is not needed in v6 and up
@@ -22,11 +22,6 @@ class OracledbPlugin extends DatabasePlugin {
       dbInstance ??= dbInfo.dbInstance
     }
 
-    if (service === undefined && hostname) {
-      // Fallback for users not providing the service properly in a serviceName method
-      service = `${hostname}:${port}/${dbInstance}`
-    }
-
     this.startSpan(this.operationName(), {
       service,
       resource: query,

packages/datadog-plugin-oracledb/test/index.spec.js

@@ -293,11 +293,11 @@ describe('Plugin', () => {
             {
               v0: {
                 opName: 'oracle.query',
-                serviceName: config.connectString
+                serviceName: 'test-oracle'
               },
               v1: {
                 opName: 'oracle.query',
-                serviceName: config.connectString
+                serviceName: 'test'
               }
             }
           )
@@ -306,7 +306,7 @@ describe('Plugin', () => {
             await Promise.all([
               agent.assertFirstTraceSpan({
                 name: expectedSchema.outbound.opName,
-                service: config.connectString
+                service: 'test-oracle'
               }),
               connection.execute(dbQuery)
             ])

packages/dd-trace/src/service-naming/schemas/v1/storage.js

@@ -14,7 +14,8 @@ const mySQLNaming = {
 
 function withFunction ({ tracerService, pluginConfig, params }) {
   if (typeof pluginConfig.service === 'function') {
-    return pluginConfig.service(params)
+    const result = pluginConfig.service(params)
+    return typeof result === 'string' && result.length > 0 ? result : tracerService
   }
   return configWithFallback({ tracerService, pluginConfig })
 }

packages/dd-trace/test/plugins/agent.js

@@ -244,7 +244,7 @@ function assertIntegrationName (args) {
             if (span && span.meta && span.meta.component && span.meta.component !== span.meta['_dd.integration']) {
               expect(span.meta['_dd.integration']).to.equal(
                 currentIntegrationName,
-                 `Expected span to have "_dd.integration" tag "${currentIntegrationName}" 
+                 `Expected span to have "_dd.integration" tag "${currentIntegrationName}"
                  but found "${span.meta['_dd.integration']}" for span ID ${span.span_id}`
               )
             }

packages/dd-trace/test/setup/mocha.js

@@ -34,7 +34,7 @@ function withNamingSchema (
   const {
     hooks = (version, defaultToGlobalService) => {},
     desc = '',
-    selectSpan = (traces) => traces[0][0]
+    selectSpan = (traces) => traces[0][0],
   } = opts
   let fullConfig