Skip to content

Commit

Permalink
modulesLoadedBeforeTrace check incorrect
Browse files Browse the repository at this point in the history
Regex now supports npm org modules, e.g. @google/cloud-trace instead of
@google - see https://regex101.com/r/lW2bE3/4 for examples.

filesLoadedBeforeTrace now ignores @google/cloud-trace instead of
@google

Also fixed double spacing.

Stops the warning:
```
WARN :@google/cloud-trace: Tracing might not work as the following modules  were loaded before the trace agent was initialized: ["@google"]
```
  • Loading branch information
Salakar authored and Matt Loring committed May 2, 2016
1 parent 1f52b8f commit 2167b16
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 8 deletions.
14 changes: 7 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ var common = require('@google/cloud-diagnostics-common');
var semver = require('semver');
var constants = require('./lib/constants.js');
var path = require('path');
var util = require('./lib/util.js');

var modulesLoadedBeforeTrace = [];
var moduleRegex =
new RegExp('.*?node_modules\\' + path.sep + '([^\\' + path.sep + ']*).*');

for (var i = 0; i < filesLoadedBeforeTrace.length; i++) {
var matches = moduleRegex.exec(filesLoadedBeforeTrace[i]);
if (matches && matches.length > 1 &&
modulesLoadedBeforeTrace.indexOf(matches[1]) === -1) {
modulesLoadedBeforeTrace.push(matches[1]);
var moduleName = util.packageNameFromPath(filesLoadedBeforeTrace[i]);
if (moduleName && moduleName !== '@google/cloud-trace' &&
modulesLoadedBeforeTrace.indexOf(moduleName) === -1) {
modulesLoadedBeforeTrace.push(moduleName);
}
}

Expand Down Expand Up @@ -146,7 +146,7 @@ var publicAgent = {

if (modulesLoadedBeforeTrace.length > 0) {
logger.warn('Tracing might not work as the following modules ' +
' were loaded before the trace agent was initialized: ' +
'were loaded before the trace agent was initialized: ' +
JSON.stringify(modulesLoadedBeforeTrace));
}

Expand Down
29 changes: 28 additions & 1 deletion lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

'use strict';

var path = require('path');

/**
* Produces an object summarization of limited size. This summarization
* does not adhere to the JSON spec so it cannot be reparsed even if
Expand Down Expand Up @@ -63,6 +65,31 @@ function stringifyPrefix(o, n) {
return buf.toString('utf8', 0, pos);
}

// Includes support for npm '@org/name' packages
// Regex: .*?node_modules\/(@[^\/]*\/[^\/]*|[^\/]*).*
// Tests: https://regex101.com/r/lW2bE3/4
var moduleRegex = new RegExp(
'.*?node_modules' + path.sep +
'(@[^' + path.sep +
']*' + path.sep +
'[^' + path.sep +
']*|[^' + path.sep +
']*).*'
);

/**
* Retrieves a package name from the full import path.
* For example:
* './node_modules/bar/index/foo.js' => 'bar'
*
* @param {string} path The full import path.
*/
function packageNameFromPath(path) {
var matches = moduleRegex.exec(path);
return matches && matches.length > 1 ? matches[1] : null;
}

module.exports = {
stringifyPrefix: stringifyPrefix
stringifyPrefix: stringifyPrefix,
packageNameFromPath: packageNameFromPath
};
13 changes: 13 additions & 0 deletions test/test-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,16 @@ describe('util.stringifyPrefix', function() {
});
});

describe('util.packageNameFromPath', function() {
it('should work for standard packages', function() {
var path = './appengine-sails/node_modules/testmodule/index.js';
assert.equal(util.packageNameFromPath(path),
'testmodule');
});

it('should work for namespaced packages', function() {
var path = './appengine-sails/node_modules/@google/cloud-trace/index.js';
assert.equal(util.packageNameFromPath(path),
'@google/cloud-trace');
});
});

0 comments on commit 2167b16

Please sign in to comment.