Skip to content

Commit 7e0b029

Browse files
authored
fix(shared-metrics): reduce memory consumption with many dependencies (#2088)
refs https://jsw.ibm.com/browse/INSTA-59265
1 parent 6ee6c33 commit 7e0b029

File tree

4 files changed

+164
-6
lines changed

4 files changed

+164
-6
lines changed

packages/shared-metrics/src/dependencies.js

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ exports.MAX_DEPENDENCIES = 750;
2727
/** @type {string} */
2828
exports.payloadPrefix = 'dependencies';
2929

30+
const MAX_DEPTH_NODE_MODULES = 2;
31+
3032
/** @type {Object.<string, string>} */
3133
const preliminaryPayload = {};
3234

@@ -87,7 +89,13 @@ exports.activate = function activate() {
8789
* @param {string} packageJsonPath
8890
*/
8991
function addAllDependencies(dependencyDir, started, packageJsonPath) {
90-
addDependenciesFromDir(dependencyDir, () => {
92+
addDependenciesFromDir(dependencyDir, 0, () => {
93+
// TODO: This check happens AFTER we have already collected the dependencies.
94+
// This is quiet useless for a large dependency tree, because we consume resources to collect
95+
// all the dependencies (fs.stats, fs.readFile etc), but then discard most of them here.
96+
// This is only critical for a very large number of defined dependencies in package.json (vertical).
97+
// NOTE: There is an extra protection in the `addDependenciesFromDir` fn to
98+
// limit the depth of traversing node_modules.
9199
if (Object.keys(preliminaryPayload).length <= exports.MAX_DEPENDENCIES) {
92100
// @ts-ignore: Cannot redeclare exported variable 'currentPayload'
93101
exports.currentPayload = preliminaryPayload;
@@ -114,7 +122,11 @@ function addAllDependencies(dependencyDir, started, packageJsonPath) {
114122
* @param {string} dependencyDir
115123
* @param {() => void} callback
116124
*/
117-
function addDependenciesFromDir(dependencyDir, callback) {
125+
function addDependenciesFromDir(dependencyDir, currentDepth = 0, callback) {
126+
if (currentDepth >= MAX_DEPTH_NODE_MODULES) {
127+
return callback();
128+
}
129+
118130
fs.readdir(dependencyDir, (readDirErr, dependencies) => {
119131
if (readDirErr || !dependencies) {
120132
logger.warn(`Cannot analyse dependencies due to ${readDirErr?.message}`);
@@ -140,11 +152,13 @@ function addDependenciesFromDir(dependencyDir, callback) {
140152

141153
filteredDependendencies.forEach(dependency => {
142154
if (dependency.indexOf('@') === 0) {
143-
addDependenciesFromDir(path.join(dependencyDir, dependency), () => {
155+
// NOTE: We do not increase currentDepth because scoped packages are just a folder containing more packages.
156+
addDependenciesFromDir(path.join(dependencyDir, dependency), currentDepth, () => {
144157
countDownLatch.countDown();
145158
});
146159
} else {
147160
const fullDirPath = path.join(dependencyDir, dependency);
161+
148162
// Only check directories. For example, yarn adds a .yarn-integrity file to /node_modules/ which we need to
149163
// exclude, otherwise we get a confusing "Failed to identify version of .yarn-integrity dependency due to:
150164
// ENOTDIR: not a directory, open '.../node_modules/.yarn-integrity/package.json'." in the logs.
@@ -159,7 +173,7 @@ function addDependenciesFromDir(dependencyDir, callback) {
159173
return;
160174
}
161175

162-
addDependency(dependency, fullDirPath, countDownLatch);
176+
addDependency(dependency, fullDirPath, countDownLatch, currentDepth);
163177
});
164178
}
165179
});
@@ -173,9 +187,11 @@ function addDependenciesFromDir(dependencyDir, callback) {
173187
* @param {string} dependency
174188
* @param {string} dependencyDirPath
175189
* @param {import('./util/CountDownLatch')} countDownLatch
190+
* @param {number} currentDepth
176191
*/
177-
function addDependency(dependency, dependencyDirPath, countDownLatch) {
192+
function addDependency(dependency, dependencyDirPath, countDownLatch, currentDepth) {
178193
const packageJsonPath = path.join(dependencyDirPath, 'package.json');
194+
179195
fs.readFile(packageJsonPath, { encoding: 'utf8' }, (err, contents) => {
180196
if (err && err.code === 'ENOENT') {
181197
// This directory does not contain a package json. This happens for example for node_modules/.cache etc.
@@ -198,19 +214,23 @@ function addDependency(dependency, dependencyDirPath, countDownLatch) {
198214
preliminaryPayload[parsedPackageJson.name] = parsedPackageJson.version;
199215
}
200216
} catch (parseErr) {
217+
// TODO: countDownLatch.countDown(); needs to be called here too?
218+
// countDownLatch.countDown();
201219
return logger.info(
202220
`Failed to identify version of ${dependency} dependency due to: ${parseErr?.message}.
203221
This means that you will not be able to see details about this dependency within Instana.`
204222
);
205223
}
206224

225+
// NOTE: The dependency metric collector does not respect if the node_modules are dev dependencies or production
226+
// dependencies. It collects all dependencies that are installed in the node_modules folder.
207227
const potentialNestedNodeModulesFolder = path.join(dependencyDirPath, 'node_modules');
208228
fs.stat(potentialNestedNodeModulesFolder, (statErr, stats) => {
209229
if (statErr || !stats.isDirectory()) {
210230
countDownLatch.countDown();
211231
return;
212232
}
213-
addDependenciesFromDir(potentialNestedNodeModulesFolder, () => {
233+
addDependenciesFromDir(potentialNestedNodeModulesFolder, currentDepth + 1, () => {
214234
countDownLatch.countDown();
215235
});
216236
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* (c) Copyright IBM Corp. 2025
3+
*/
4+
5+
'use strict';
6+
7+
// NOTE: c8 bug https://github.com/bcoe/c8/issues/166
8+
process.on('SIGTERM', () => {
9+
process.disconnect();
10+
process.exit(0);
11+
});
12+
13+
const instana = require('@instana/collector');
14+
instana();
15+
16+
const express = require('express');
17+
18+
const logPrefix = `Many dependencies app (${process.pid}):\t`;
19+
20+
const app = express();
21+
22+
app.get('/', (req, res) => res.sendStatus(200));
23+
24+
app.listen(process.env.APP_PORT, () => {
25+
log(`Listening on port: ${process.env.APP_PORT}`);
26+
});
27+
28+
function log() {
29+
const args = Array.prototype.slice.call(arguments);
30+
args[0] = logPrefix + args[0];
31+
// eslint-disable-next-line no-console
32+
console.log.apply(console, args);
33+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
{
2+
"name": "app-with-many-dependencies",
3+
"version": "1.0.0",
4+
"description": "",
5+
"main": "index.js",
6+
"scripts": {
7+
"test": "echo \"Error: no test specified\" && exit 1"
8+
},
9+
"keywords": [],
10+
"author": "",
11+
"license": "ISC",
12+
"type": "commonjs",
13+
"dependencies": {
14+
"@angular/animations": "20.0.3",
15+
"@angular/cdk": "20.0.3",
16+
"@angular/common": "20.0.3",
17+
"@angular/compiler": "20.0.3",
18+
"@angular/core": "20.0.3",
19+
"@angular/forms": "20.0.3",
20+
"@angular/material": "20.0.3",
21+
"@angular/material-moment-adapter": "20.0.3",
22+
"@angular/platform-browser": "20.0.3",
23+
"@angular/platform-browser-dynamic": "20.0.3",
24+
"@angular/router": "20.0.3",
25+
"@ng-matero/extensions": "20.0.3",
26+
"angular13-organization-chart": "2.0.0",
27+
"bcm-in-frontend-v2": "file:",
28+
"bcm-ph-frontend": "file:",
29+
"chart.js": "4.4.1",
30+
"chartjs-plugin-zoom": "2.0.1",
31+
"compression": "1.8.1",
32+
"dayjs": "1.11.7",
33+
"dotenv": "16.4.5",
34+
"express": "4.21.2",
35+
"file-saver": "2.0.5",
36+
"flatpickr": "4.6.13",
37+
"http-proxy": "1.18.1",
38+
"ibmcloud-appid-js": "1.0.1",
39+
"keen-slider": "6.8.6",
40+
"leaflet": "1.9.3",
41+
"lodash-es": "4.17.21",
42+
"moment": "2.30.1",
43+
"path-to-regexp": "0.1.12",
44+
"rxjs": "7.8.1",
45+
"tslib": "2.3.0",
46+
"xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz",
47+
"zone.js": "0.15.1"
48+
},
49+
"devDependencies": {
50+
"@angular-devkit/build-angular": "20.0.2",
51+
"@angular/cli": "19.2.15",
52+
"@angular/compiler-cli": "20.0.3",
53+
"@schematics/angular": "18.2.3",
54+
"@types/leaflet": "1.9.0",
55+
"@types/lodash-es": "4.17.7",
56+
"typescript": "5.8.3"
57+
}
58+
}

packages/shared-metrics/test/dependencies/test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,53 @@ describe('dependencies', function () {
212212
})
213213
));
214214
});
215+
216+
describe('with many dependencies', () => {
217+
const appDir = path.join(__dirname, 'app-with-many-dependencies');
218+
219+
let controls;
220+
221+
before(async () => {
222+
// eslint-disable-next-line no-console
223+
console.log('Installing dependencies for app-with-many-dependencies. This may take a while...');
224+
runCommandSync('rm -rf node_modules', appDir);
225+
runCommandSync('npm install --no-optional --no-audit --no-package-lock', appDir);
226+
// eslint-disable-next-line no-console
227+
console.log('Installed dependencies for app-with-many-dependencies');
228+
controls = new ProcessControls({
229+
dirname: appDir,
230+
useGlobalAgent: true
231+
});
232+
233+
await controls.startAndWaitForAgentConnection();
234+
});
235+
236+
before(async () => {
237+
await agentControls.clearReceivedTraceData();
238+
});
239+
240+
after(async () => {
241+
await controls.stop();
242+
});
243+
244+
it('should limit dependencies', () => {
245+
return retry(async () => {
246+
const allMetrics = await agentControls.getAllMetrics(controls.getPid());
247+
expect(allMetrics).to.be.an('array');
248+
249+
const deps = findMetric(allMetrics, ['dependencies']);
250+
expect(deps).to.be.an('object');
251+
expect(Object.keys(deps).length).to.be.at.least(500);
252+
expect(Object.keys(deps).length).to.be.at.most(1000);
253+
254+
// expect that the first dependency is in the list
255+
expect(deps['@ampproject/remapping']).to.exist;
256+
257+
// expect that the last dependency is in the list
258+
expect(deps['zone.js']).to.exist;
259+
});
260+
});
261+
});
215262
});
216263

217264
/**

0 commit comments

Comments
 (0)