Fix same dependency detection (#3386)

* Fix same dependency detection

* Do not send same dependency:version twice

* Clear map on stop

* Update savedDependencies to savedDependenciesToSend to improve understanding

Author: uurien

packages/dd-trace/src/telemetry/dependencies.js

@@ -7,8 +7,10 @@ const { sendData } = require('./send-data')
 const dc = require('../../../diagnostics_channel')
 const { fileURLToPath } = require('url')
 
-const savedDependencies = new Set()
-const detectedDependencyNames = new Set()
+const savedDependenciesToSend = new Set()
+const detectedDependencyKeys = new Set()
+const detectedDependencyVersions = new Set()
+
 const FILE_URI_START = `file://`
 const moduleLoadStartChannel = dc.channel('dd-trace:moduleLoadStart')
 
@@ -18,14 +20,14 @@ function waitAndSend (config, application, host) {
   if (!immediate) {
     immediate = setImmediate(() => {
       immediate = null
-      if (savedDependencies.size > 0) {
-        const dependencies = Array.from(savedDependencies.values()).splice(0, 1000).map(pair => {
-          savedDependencies.delete(pair)
+      if (savedDependenciesToSend.size > 0) {
+        const dependencies = Array.from(savedDependenciesToSend.values()).splice(0, 1000).map(pair => {
+          savedDependenciesToSend.delete(pair)
           const [name, version] = pair.split(' ')
           return { name, version }
         })
         sendData(config, application, host, 'app-dependencies-loaded', { dependencies })
-        if (savedDependencies.size > 0) {
+        if (savedDependenciesToSend.size > 0) {
           waitAndSend(config, application, host)
         }
       }
@@ -46,15 +48,24 @@ function onModuleLoad (data) {
     }
     const parseResult = filename && parse(filename)
     const request = data.request || (parseResult && parseResult.name)
-    if (filename && request && isDependency(filename, request) && !detectedDependencyNames.has(request)) {
-      detectedDependencyNames.add(request)
+    const dependencyKey = parseResult && parseResult.basedir ? parseResult.basedir : request
+
+    if (filename && request && isDependency(filename, request) && !detectedDependencyKeys.has(dependencyKey)) {
+      detectedDependencyKeys.add(dependencyKey)
+
       if (parseResult) {
         const { name, basedir } = parseResult
         if (basedir) {
           try {
             const { version } = requirePackageJson(basedir, module)
-            savedDependencies.add(`${name} ${version}`)
-            waitAndSend(config, application, host)
+            const dependencyAndVersion = `${name} ${version}`
+
+            if (!detectedDependencyVersions.has(dependencyAndVersion)) {
+              savedDependenciesToSend.add(dependencyAndVersion)
+              detectedDependencyVersions.add(dependencyAndVersion)
+
+              waitAndSend(config, application, host)
+            }
           } catch (e) {
             // can not read the package.json, do nothing
           }
@@ -88,8 +99,9 @@ function stop () {
   config = null
   application = null
   host = null
-  detectedDependencyNames.clear()
-  savedDependencies.clear()
+  detectedDependencyKeys.clear()
+  savedDependenciesToSend.clear()
+  detectedDependencyVersions.clear()
   if (moduleLoadStartChannel.hasSubscribers) {
     moduleLoadStartChannel.unsubscribe(onModuleLoad)
   }

packages/dd-trace/test/telemetry/dependencies.spec.js

@@ -198,6 +198,73 @@ describe('dependencies', () => {
         .to.have.been.calledOnceWith(config, application, host, 'app-dependencies-loaded', expectedDependencies)
     })
 
+    it('should include two dependencies when they are in different paths', () => {
+      const moduleName = 'custom-module'
+      const packageVersion = '1.0.0'
+      const nestedPackageVersion = '0.5.0'
+      const firstLevelDependency = [fileURIWithoutNodeModules, 'node_modules', moduleName, 'index1.js'].join('/')
+      const nestedDependency =
+        [fileURIWithoutNodeModules, 'node_modules', 'dependency', 'node_modules', moduleName, 'index1.js'].join('/')
+
+      requirePackageJson.callsFake(function (dependencyPath) {
+        if (dependencyPath.includes(path.join('node_modules', 'dependency', 'node_modules'))) {
+          return { version: nestedPackageVersion }
+        } else {
+          return { version: packageVersion }
+        }
+      })
+
+      dependencies.start(config, application, host)
+      moduleLoadStartChannel.publish({ request: moduleName, filename: firstLevelDependency })
+      moduleLoadStartChannel.publish({ request: moduleName, filename: nestedDependency })
+
+      const expectedDependencies1 = {
+        dependencies: [
+          { name: moduleName, version: packageVersion }
+        ]
+      }
+      const expectedDependencies2 = {
+        dependencies: [
+          { name: moduleName, version: nestedPackageVersion }
+        ]
+      }
+      expect(sendData).to.have.been.calledTwice
+
+      expect(sendData.firstCall)
+        .to.have.been.calledWith(config, application, host, 'app-dependencies-loaded', expectedDependencies1)
+
+      expect(sendData.secondCall)
+        .to.have.been.calledWith(config, application, host, 'app-dependencies-loaded', expectedDependencies2)
+    })
+
+    it('should include only one dependency when they are in different paths but the version number is the same', () => {
+      const moduleName = 'custom-module'
+      const packageVersion = '1.0.0'
+      const firstLevelDependency = [fileURIWithoutNodeModules, 'node_modules', moduleName, 'index1.js'].join('/')
+      const nestedDependency =
+        [fileURIWithoutNodeModules, 'node_modules', 'dependency', 'node_modules', moduleName, 'index1.js'].join('/')
+
+      requirePackageJson.callsFake(function (dependencyPath) {
+        if (dependencyPath.includes(path.join('node_modules', 'dependency', 'node_modules'))) {
+          return { version: packageVersion }
+        } else {
+          return { version: packageVersion }
+        }
+      })
+
+      dependencies.start(config, application, host)
+      moduleLoadStartChannel.publish({ request: moduleName, filename: firstLevelDependency })
+      moduleLoadStartChannel.publish({ request: moduleName, filename: nestedDependency })
+
+      const expectedDependencies = {
+        dependencies: [
+          { name: moduleName, version: packageVersion }
+        ]
+      }
+      expect(sendData).to.have.been
+        .calledOnceWith(config, application, host, 'app-dependencies-loaded', expectedDependencies)
+    })
+
     it('should call sendData only once with duplicated dependency', () => {
       const request = 'custom-module'
       requirePackageJson.returns({ version: '1.0.0' })