fix: optimized how we receive the package.json information #2176
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| ####################################### | ||
| # (c) Copyright IBM Corp. 2025 | ||
| ####################################### | ||
|
|
||
| cd "$(dirname "$0")/../../../collector" | ||
| echo "Running npm pack in $(pwd)" | ||
| npm pack | ||
|
|
||
| version=$(node -p "require('./package.json').version") | ||
| tarball="instana-collector-${version}.tgz" | ||
| cp "./${tarball}" "./test/nativeModuleRetry/collector.tgz" | ||
|
|
||
| cd "$(dirname "$0")/../core" | ||
| echo "Running npm pack in $(pwd)" | ||
| npm pack | ||
|
|
||
| version=$(node -p "require('./package.json').version") | ||
| tarball="instana-core-${version}.tgz" | ||
| cp "./${tarball}" "../collector/test/nativeModuleRetry/core.tgz" | ||
|
|
||
| cd "$(dirname "$0")/../shared-metrics" | ||
| echo "Running npm pack in $(pwd)" | ||
| npm pack | ||
|
|
||
| version=$(node -p "require('./package.json').version") | ||
| tarball="instana-shared-metrics-${version}.tgz" | ||
| cp "./${tarball}" "../collector/test/nativeModuleRetry/shared-metrics.tgz" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * (c) Copyright IBM Corp. 2025 | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const path = require('path'); | ||
| const childProcess = require('child_process'); | ||
| const expect = require('chai').expect; | ||
| const supportedVersion = require('@instana/core').tracing.supportedVersion; | ||
| const config = require('../../../../../core/test/config'); | ||
| const mochaSuiteFn = supportedVersion(process.versions.node) ? describe : describe.skip; | ||
|
|
||
| mochaSuiteFn('tracing/invalidApp', function () { | ||
| this.timeout(config.getTestTimeout() * 3); | ||
|
|
||
| it('when the collector is required in interactive shell', cb => { | ||
| const child = childProcess.spawn( | ||
| process.execPath, | ||
| ['--require', path.join(__dirname, '..', '..', '..', '..', 'src', 'immediate.js')], | ||
| { | ||
| stdio: 'inherit', | ||
| cwd: process.cwd(), | ||
| env: process.env | ||
| } | ||
| ); | ||
|
|
||
| child.on('error', err => { | ||
| cb(err); | ||
| }); | ||
|
|
||
| child.on('exit', (code, signal) => { | ||
| expect(signal).to.equal('SIGTERM'); | ||
| expect(code).to.not.exist; | ||
| cb(); | ||
| }); | ||
|
|
||
| setTimeout(() => { | ||
| child.kill('SIGTERM'); | ||
| }, 3 * 1000); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,10 @@ function isAppInstalledIntoNodeModules() { | |
| return appInstalledIntoNodeModules; | ||
| } | ||
|
|
||
| const MAX_ATTEMPTS = 5; | ||
| const DELAY = 1500; | ||
| let attempts = 0; | ||
|
Contributor
There was a problem hiding this comment. Centralized retries (see name.js and version.js etc.). |
||
|
|
||
| /** | ||
| * Looks for the app's main package.json file, parses it and returns the parsed content. The search is started at | ||
| * path.dirname(require.main.filename). | ||
|
|
@@ -50,15 +54,31 @@ function isAppInstalledIntoNodeModules() { | |
| function getMainPackageJsonStartingAtMainModule(cb) { | ||
| // NOTE: we already cached the package.json | ||
| if (parsedMainPackageJson !== undefined) { | ||
| return cb(null, { file: parsedMainPackageJson, path: mainPackageJsonPath }); | ||
|
Contributor
There was a problem hiding this comment. Always return both file + path. Some metrics need the file, some the path. |
||
| } | ||
|
|
||
| // CASE: customer provided custom package.json path, let's try loading it | ||
| if (packageJsonPath) { | ||
| return readFile(packageJsonPath, cb); | ||
| } | ||
|
|
||
| return getMainPackageJsonStartingAtDirectory(null, (err, pkg) => { | ||
| // CASE: using --require or --import can result in an empty require.main initially | ||
| if (err && attempts < MAX_ATTEMPTS) { | ||
| attempts++; | ||
|
|
||
| logger.warn( | ||
| `Could not determine main package.json yet. Retrying ${attempts}/${MAX_ATTEMPTS} after ${DELAY}ms...` | ||
| ); | ||
|
|
||
| return setTimeout(() => { | ||
| getMainPackageJsonStartingAtMainModule(cb); | ||
| }, DELAY).unref(); | ||
| } | ||
|
|
||
| attempts = 0; | ||
| return cb(err, pkg); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -110,22 +130,10 @@ function readFile(filePath, cb) { | |
| return cb(e, null); | ||
| } | ||
|
|
||
| return cb(null, { file: parsedMainPackageJson, path: filePath }); | ||
| }); | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Not needed anymore, because we now request pkg json + pkg json path centralized in core metrics index.js. We always provide the found path and the file to the metric. |
||
| /** | ||
| * Looks for path of the app's main package.json file, starting the search at the given directory. If the given | ||
| * directory is null or undefined, the search will start at path.dirname(require.main.filename). | ||
|
|
@@ -187,6 +195,12 @@ function getMainPackageJsonPathStartingAtDirectory(startDirectory, cb) { | |
| } | ||
| } | ||
|
|
||
| // CASE: node --require .../src/immediate.js | ||
| if (!mainModule?.filename) { | ||
| const err = new Error('Application entrypoint could not be identified.'); | ||
| return process.nextTick(cb, err, null); | ||
|
Contributor
There was a problem hiding this comment. This is the actual fix. @aryamohanan started with it already. |
||
| } | ||
|
|
||
| startDirectory = path.dirname(mainModule.filename); | ||
| } | ||
|
|
||
|
|
@@ -332,14 +346,14 @@ function searchInParentDir(dir, onParentDir, cb) { | |
| const reset = () => { | ||
| parsedMainPackageJson = undefined; | ||
| mainPackageJsonPath = undefined; | ||
| attempts = 0; | ||
| }; | ||
|
|
||
| module.exports = { | ||
| init, | ||
| isAppInstalledIntoNodeModules, | ||
| getMainPackageJsonStartingAtMainModule, | ||
| getMainPackageJsonStartingAtDirectory, | ||
| getMainPackageJsonPathStartingAtDirectory, | ||
| findNodeModulesFolder, | ||
| reset | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will outsource to main. Test was failing because shared-metrics was not installed from local folder - it took the version from NPM - this does not work if you do refactorings (such as removing fns)