From 2167b16941b89a33aed6d9e3bebc3f9e9cb534d0 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Mon, 2 May 2016 14:17:46 +0100 Subject: [PATCH] modulesLoadedBeforeTrace check incorrect 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"] ``` --- index.js | 14 +++++++------- lib/util.js | 29 ++++++++++++++++++++++++++++- test/test-util.js | 13 +++++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index 5c846edeb..bab863045 100644 --- a/index.js +++ b/index.js @@ -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); } } @@ -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)); } diff --git a/lib/util.js b/lib/util.js index d7c33248a..91cab01e1 100644 --- a/lib/util.js +++ b/lib/util.js @@ -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 @@ -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 }; diff --git a/test/test-util.js b/test/test-util.js index 0989b44d0..99d05d54a 100644 --- a/test/test-util.js +++ b/test/test-util.js @@ -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'); + }); +});