diff --git a/packages/shared-metrics/src/dependencies.js b/packages/shared-metrics/src/dependencies.js index a6c1aaba76..71eb49dd26 100644 --- a/packages/shared-metrics/src/dependencies.js +++ b/packages/shared-metrics/src/dependencies.js @@ -27,8 +27,10 @@ exports.MAX_DEPENDENCIES = 750; /** @type {string} */ exports.payloadPrefix = 'dependencies'; +const MAX_DEPTH_NODE_MODULES = 2; + /** @type {Object.} */ -const preliminaryPayload = {}; +let preliminaryPayload = {}; /** @type {Object.} */ // @ts-ignore: Cannot redeclare exported variable 'currentPayload' @@ -40,6 +42,9 @@ const DELAY = 1000; let attempts = 0; exports.activate = function activate() { + // Ensure we start fresh each time activate is called + preliminaryPayload = {}; + attempts++; const started = Date.now(); @@ -56,7 +61,9 @@ exports.activate = function activate() { ); util.applicationUnderMonitoring.findNodeModulesFolder((errNodeModules, nodeModulesFolder) => { if (errNodeModules) { - return logger.warn(`Failed to determine node_modules folder. Reason: ${err?.message}, ${err?.stack}`); + return logger.warn( + `Failed to determine node_modules folder. Reason: ${errNodeModules?.message}, ${errNodeModules?.stack}` + ); } else if (!nodeModulesFolder) { return logger.warn( 'Neither the package.json file nor the node_modules folder could be found. Stopping dependency analysis.' @@ -87,7 +94,13 @@ exports.activate = function activate() { * @param {string} packageJsonPath */ function addAllDependencies(dependencyDir, started, packageJsonPath) { - addDependenciesFromDir(dependencyDir, () => { + addDependenciesFromDir(dependencyDir, 0, () => { + // TODO: This check happens AFTER we have already collected the dependencies. + // This is quiet useless for a large dependency tree, because we consume resources to collect + // all the dependencies (fs.stats, fs.readFile etc), but then discard most of them here. + // This is only critical for a very large package.json. + // NOTE: There is an extra protection in the `addDependenciesFromDir` fn to + // limit the depth of traversing node_modules. if (Object.keys(preliminaryPayload).length <= exports.MAX_DEPENDENCIES) { // @ts-ignore: Cannot redeclare exported variable 'currentPayload' exports.currentPayload = preliminaryPayload; @@ -114,7 +127,11 @@ function addAllDependencies(dependencyDir, started, packageJsonPath) { * @param {string} dependencyDir * @param {() => void} callback */ -function addDependenciesFromDir(dependencyDir, callback) { +function addDependenciesFromDir(dependencyDir, currentDepth = 0, callback) { + if (currentDepth >= MAX_DEPTH_NODE_MODULES) { + return callback(); + } + fs.readdir(dependencyDir, (readDirErr, dependencies) => { if (readDirErr || !dependencies) { logger.warn(`Cannot analyse dependencies due to ${readDirErr?.message}`); @@ -132,7 +149,8 @@ function addDependenciesFromDir(dependencyDir, callback) { return; } - // This latch fires once all dependencies of the current directory in the node_modules tree have been analysed. + // NOTE: `filteredDependendencies.length` is the length of the current directory only. + // The latch counts down as we finish processing each dependency in THIS current directory. const countDownLatch = new CountDownLatch(filteredDependendencies.length); countDownLatch.once('done', () => { callback(); @@ -140,11 +158,12 @@ function addDependenciesFromDir(dependencyDir, callback) { filteredDependendencies.forEach(dependency => { if (dependency.indexOf('@') === 0) { - addDependenciesFromDir(path.join(dependencyDir, dependency), () => { + addDependenciesFromDir(path.join(dependencyDir, dependency), currentDepth + 1, () => { countDownLatch.countDown(); }); } else { const fullDirPath = path.join(dependencyDir, dependency); + // Only check directories. For example, yarn adds a .yarn-integrity file to /node_modules/ which we need to // exclude, otherwise we get a confusing "Failed to identify version of .yarn-integrity dependency due to: // ENOTDIR: not a directory, open '.../node_modules/.yarn-integrity/package.json'." in the logs. @@ -159,7 +178,7 @@ function addDependenciesFromDir(dependencyDir, callback) { return; } - addDependency(dependency, fullDirPath, countDownLatch); + addDependency(dependency, fullDirPath, countDownLatch, currentDepth); }); } }); @@ -173,9 +192,11 @@ function addDependenciesFromDir(dependencyDir, callback) { * @param {string} dependency * @param {string} dependencyDirPath * @param {import('./util/CountDownLatch')} countDownLatch + * @param {number} currentDepth */ -function addDependency(dependency, dependencyDirPath, countDownLatch) { +function addDependency(dependency, dependencyDirPath, countDownLatch, currentDepth) { const packageJsonPath = path.join(dependencyDirPath, 'package.json'); + fs.readFile(packageJsonPath, { encoding: 'utf8' }, (err, contents) => { if (err && err.code === 'ENOENT') { // This directory does not contain a package json. This happens for example for node_modules/.cache etc. @@ -198,19 +219,22 @@ function addDependency(dependency, dependencyDirPath, countDownLatch) { preliminaryPayload[parsedPackageJson.name] = parsedPackageJson.version; } } catch (parseErr) { + countDownLatch.countDown(); return logger.info( `Failed to identify version of ${dependency} dependency due to: ${parseErr?.message}. This means that you will not be able to see details about this dependency within Instana.` ); } + // NOTE: The dependency metric collector does not respect if the node_modules are dev dependencies or production + // dependencies. It collects all dependencies that are installed in the node_modules folder. const potentialNestedNodeModulesFolder = path.join(dependencyDirPath, 'node_modules'); fs.stat(potentialNestedNodeModulesFolder, (statErr, stats) => { if (statErr || !stats.isDirectory()) { countDownLatch.countDown(); return; } - addDependenciesFromDir(potentialNestedNodeModulesFolder, () => { + addDependenciesFromDir(potentialNestedNodeModulesFolder, currentDepth + 1, () => { countDownLatch.countDown(); }); }); diff --git a/packages/shared-metrics/test/dependencies/app-with-many-dependencies/app.js b/packages/shared-metrics/test/dependencies/app-with-many-dependencies/app.js new file mode 100644 index 0000000000..563dcaf941 --- /dev/null +++ b/packages/shared-metrics/test/dependencies/app-with-many-dependencies/app.js @@ -0,0 +1,33 @@ +/* + * (c) Copyright IBM Corp. 2025 + */ + +'use strict'; + +// NOTE: c8 bug https://github.com/bcoe/c8/issues/166 +process.on('SIGTERM', () => { + process.disconnect(); + process.exit(0); +}); + +const instana = require('@instana/collector'); +instana(); + +const express = require('express'); + +const logPrefix = `Many dependencies app (${process.pid}):\t`; + +const app = express(); + +app.get('/', (req, res) => res.sendStatus(200)); + +app.listen(process.env.APP_PORT, () => { + log(`Listening on port: ${process.env.APP_PORT}`); +}); + +function log() { + const args = Array.prototype.slice.call(arguments); + args[0] = logPrefix + args[0]; + // eslint-disable-next-line no-console + console.log.apply(console, args); +} diff --git a/packages/shared-metrics/test/dependencies/app-with-many-dependencies/package.json b/packages/shared-metrics/test/dependencies/app-with-many-dependencies/package.json new file mode 100644 index 0000000000..8896f0e70a --- /dev/null +++ b/packages/shared-metrics/test/dependencies/app-with-many-dependencies/package.json @@ -0,0 +1,58 @@ +{ + "name": "app-with-many-dependencies", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "ISC", + "type": "commonjs", + "dependencies": { + "@angular/animations": "20.0.3", + "@angular/cdk": "20.0.3", + "@angular/common": "20.0.3", + "@angular/compiler": "20.0.3", + "@angular/core": "20.0.3", + "@angular/forms": "20.0.3", + "@angular/material": "20.0.3", + "@angular/material-moment-adapter": "20.0.3", + "@angular/platform-browser": "20.0.3", + "@angular/platform-browser-dynamic": "20.0.3", + "@angular/router": "20.0.3", + "@ng-matero/extensions": "20.0.3", + "angular13-organization-chart": "2.0.0", + "bcm-in-frontend-v2": "file:", + "bcm-ph-frontend": "file:", + "chart.js": "4.4.1", + "chartjs-plugin-zoom": "2.0.1", + "compression": "1.8.1", + "dayjs": "1.11.7", + "dotenv": "16.4.5", + "express": "4.21.2", + "file-saver": "2.0.5", + "flatpickr": "4.6.13", + "http-proxy": "1.18.1", + "ibmcloud-appid-js": "1.0.1", + "keen-slider": "6.8.6", + "leaflet": "1.9.3", + "lodash-es": "4.17.21", + "moment": "2.30.1", + "path-to-regexp": "0.1.12", + "rxjs": "7.8.1", + "tslib": "2.3.0", + "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz", + "zone.js": "0.15.1" + }, + "devDependencies": { + "@angular-devkit/build-angular": "20.0.2", + "@angular/cli": "19.2.15", + "@angular/compiler-cli": "20.0.3", + "@schematics/angular": "18.2.3", + "@types/leaflet": "1.9.0", + "@types/lodash-es": "4.17.7", + "typescript": "5.8.3" + } +} diff --git a/packages/shared-metrics/test/dependencies/test.js b/packages/shared-metrics/test/dependencies/test.js index bbecf08d26..f2ce3ec7eb 100644 --- a/packages/shared-metrics/test/dependencies/test.js +++ b/packages/shared-metrics/test/dependencies/test.js @@ -212,6 +212,53 @@ describe('dependencies', function () { }) )); }); + + describe('with many dependencies', () => { + const appDir = path.join(__dirname, 'app-with-many-dependencies'); + + let controls; + + before(async () => { + // eslint-disable-next-line no-console + console.log('Installing dependencies for app-with-many-dependencies. This may take a while...'); + runCommandSync('rm -rf node_modules', appDir); + runCommandSync('npm install --no-optional --no-audit --no-package-lock', appDir); + // eslint-disable-next-line no-console + console.log('Installed dependencies for app-with-many-dependencies'); + controls = new ProcessControls({ + dirname: appDir, + useGlobalAgent: true + }); + + await controls.startAndWaitForAgentConnection(); + }); + + before(async () => { + await agentControls.clearReceivedTraceData(); + }); + + after(async () => { + await controls.stop(); + }); + + it('should limit dependencies', () => { + return retry(async () => { + const allMetrics = await agentControls.getAllMetrics(controls.getPid()); + expect(allMetrics).to.be.an('array'); + + const deps = findMetric(allMetrics, ['dependencies']); + expect(deps).to.be.an('object'); + expect(Object.keys(deps).length).to.be.at.least(500); + expect(Object.keys(deps).length).to.be.at.most(1000); + + // expect that the first dependency is in the list + expect(deps['@ampproject/remapping']).to.exist; + + // expect that the last dependency is in the list + expect(deps['zone.js']).to.exist; + }); + }); + }); }); /**