From 860458adc87a6d5b8ce5cf6a8f74a3830356da44 Mon Sep 17 00:00:00 2001 From: "went.out" Date: Thu, 11 Apr 2019 18:21:22 +0300 Subject: [PATCH 1/8] Seems this doesn't matter anymore https://github.com/nearform/node-clinic-doctor/pull/132 But ths still relates to current state https://github.com/nearform/node-clinic/issues/148 So there might be changes done to fix NODE_PATH back, for legacy projects. --- index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 1ca1b560..c1de52e9 100644 --- a/index.js +++ b/index.js @@ -24,6 +24,10 @@ const buildJs = require('@nearform/clinic-common/scripts/build-js') const buildCss = require('@nearform/clinic-common/scripts/build-css') const mainTemplate = require('@nearform/clinic-common/templates/main') +const makeInjectPath = (fileName) => { + return path.join(__dirname, 'injects', fileName); +}; + class ClinicDoctor extends events.EventEmitter { constructor (settings = {}) { super() @@ -45,21 +49,19 @@ class ClinicDoctor extends events.EventEmitter { collect (args, callback) { // run program, but inject the sampler const logArgs = [ - '-r', 'no-cluster.js', - '-r', 'sampler.js', + '-r', makeInjectPath('no-cluster.js'), + '-r', makeInjectPath('logger.js'), '--trace-events-enabled', '--trace-event-categories', 'v8' ] const stdio = ['inherit', 'inherit', 'inherit'] if (this.detectPort) { - logArgs.push('-r', 'detect-port.js') + logArgs.push('-r', makeInjectPath('detect-port.js')) stdio.push('pipe') } const customEnv = { - // use NODE_PATH to work around issues with spaces in inject path - NODE_PATH: path.join(__dirname, 'injects'), NODE_OPTIONS: logArgs.join(' ') + ( process.env.NODE_OPTIONS ? ' ' + process.env.NODE_OPTIONS : '' ), From cfd558947258bd88f75a31118e13d8e1147b9255 Mon Sep 17 00:00:00 2001 From: "went.out" Date: Thu, 11 Apr 2019 19:30:32 +0300 Subject: [PATCH 2/8] fix CI comma & occasionally replaced logger path --- index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index c1de52e9..5eb8560e 100644 --- a/index.js +++ b/index.js @@ -25,8 +25,8 @@ const buildCss = require('@nearform/clinic-common/scripts/build-css') const mainTemplate = require('@nearform/clinic-common/templates/main') const makeInjectPath = (fileName) => { - return path.join(__dirname, 'injects', fileName); -}; + return path.join(__dirname, 'injects', fileName) +} class ClinicDoctor extends events.EventEmitter { constructor (settings = {}) { @@ -50,7 +50,7 @@ class ClinicDoctor extends events.EventEmitter { // run program, but inject the sampler const logArgs = [ '-r', makeInjectPath('no-cluster.js'), - '-r', makeInjectPath('logger.js'), + '-r', makeInjectPath('sampler.js'), '--trace-events-enabled', '--trace-event-categories', 'v8' ] From ee226c3371e5adafc67d3ee9dd97677c99bb4f7b Mon Sep 17 00:00:00 2001 From: "went.out" Date: Thu, 11 Apr 2019 20:04:46 +0300 Subject: [PATCH 3/8] onliner for makeInjectPath --- index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/index.js b/index.js index 5eb8560e..0cdeb696 100644 --- a/index.js +++ b/index.js @@ -24,9 +24,7 @@ const buildJs = require('@nearform/clinic-common/scripts/build-js') const buildCss = require('@nearform/clinic-common/scripts/build-css') const mainTemplate = require('@nearform/clinic-common/templates/main') -const makeInjectPath = (fileName) => { - return path.join(__dirname, 'injects', fileName) -} +const makeInjectPath = fileName => path.join(__dirname, 'injects', fileName) class ClinicDoctor extends events.EventEmitter { constructor (settings = {}) { From c4e9c498cf200b21620ea59782f7413d5e3df7ee Mon Sep 17 00:00:00 2001 From: "went.out" Date: Tue, 23 Apr 2019 20:42:31 +0300 Subject: [PATCH 4/8] Additional Fix after code-review. Thanks to @goto-bus-stop ! NODE_PATH works from both prospects. --- index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/index.js b/index.js index 0cdeb696..23f51ec2 100644 --- a/index.js +++ b/index.js @@ -59,7 +59,14 @@ class ClinicDoctor extends events.EventEmitter { stdio.push('pipe') } + let NODE_PATH = path.join(__dirname, 'injects') + // use NODE_PATH to work around issues with spaces in inject path + if (process.env.NODE_PATH) { + NODE_PATH += `${process.platform === 'win32' ? ';' : ':'}${process.env.NODE_PATH}` + } + const customEnv = { + NODE_PATH, NODE_OPTIONS: logArgs.join(' ') + ( process.env.NODE_OPTIONS ? ' ' + process.env.NODE_OPTIONS : '' ), From 3ff566f5bd9434124b5a741b43fea8707e8b3350 Mon Sep 17 00:00:00 2001 From: "went.out" Date: Wed, 24 Apr 2019 00:54:15 +0300 Subject: [PATCH 5/8] Finally got rid of broken parts. --- index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 23f51ec2..425a1e41 100644 --- a/index.js +++ b/index.js @@ -24,8 +24,6 @@ const buildJs = require('@nearform/clinic-common/scripts/build-js') const buildCss = require('@nearform/clinic-common/scripts/build-css') const mainTemplate = require('@nearform/clinic-common/templates/main') -const makeInjectPath = fileName => path.join(__dirname, 'injects', fileName) - class ClinicDoctor extends events.EventEmitter { constructor (settings = {}) { super() @@ -47,15 +45,15 @@ class ClinicDoctor extends events.EventEmitter { collect (args, callback) { // run program, but inject the sampler const logArgs = [ - '-r', makeInjectPath('no-cluster.js'), - '-r', makeInjectPath('sampler.js'), + '-r', 'no-cluster.js', + '-r', 'sampler.js', '--trace-events-enabled', '--trace-event-categories', 'v8' ] const stdio = ['inherit', 'inherit', 'inherit'] if (this.detectPort) { - logArgs.push('-r', makeInjectPath('detect-port.js')) + logArgs.push('-r', 'detect-port.js') stdio.push('pipe') } @@ -66,6 +64,7 @@ class ClinicDoctor extends events.EventEmitter { } const customEnv = { + // use NODE_PATH to work around issues with spaces in inject path NODE_PATH, NODE_OPTIONS: logArgs.join(' ') + ( process.env.NODE_OPTIONS ? ' ' + process.env.NODE_OPTIONS : '' From f46723623e3afe2bec68bbd62c96618d48e40f89 Mon Sep 17 00:00:00 2001 From: "went.out" Date: Wed, 24 Apr 2019 01:20:19 +0300 Subject: [PATCH 6/8] investigation checks issues --- test/cmd-collect.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/cmd-collect.test.js b/test/cmd-collect.test.js index 64097444..393d6c14 100644 --- a/test/cmd-collect.test.js +++ b/test/cmd-collect.test.js @@ -88,10 +88,14 @@ test('cmd - collect - data files have content', function (t) { if (err) return t.ifError(err) // expect time seperation to be 10ms, allow 20ms error + const allow = 20 //ms const sampleTimes = output.processStat.map((stat) => stat.timestamp) const timeSeperation = summary(diff(sampleTimes)).mean() t.ok(sampleTimes.length > 0, 'data is outputted') - t.ok(Math.abs(timeSeperation - 10) < 20) + const abs = Math.abs(timeSeperation - 10); + const difference = abs - allow; + process._rawDebug(abs, difference); + t.ok(difference < 0); t.end() }) From e34ecd32e24bafb0069b14342fc900ddf86137f2 Mon Sep 17 00:00:00 2001 From: "went.out" Date: Wed, 24 Apr 2019 01:28:15 +0300 Subject: [PATCH 7/8] investigation checks issues 2 --- test/cmd-collect.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/cmd-collect.test.js b/test/cmd-collect.test.js index 393d6c14..2d27790d 100644 --- a/test/cmd-collect.test.js +++ b/test/cmd-collect.test.js @@ -88,14 +88,14 @@ test('cmd - collect - data files have content', function (t) { if (err) return t.ifError(err) // expect time seperation to be 10ms, allow 20ms error - const allow = 20 //ms + const allow = 20 const sampleTimes = output.processStat.map((stat) => stat.timestamp) const timeSeperation = summary(diff(sampleTimes)).mean() t.ok(sampleTimes.length > 0, 'data is outputted') - const abs = Math.abs(timeSeperation - 10); - const difference = abs - allow; - process._rawDebug(abs, difference); - t.ok(difference < 0); + const abs = Math.abs(timeSeperation - 10) + const difference = abs - allow + process._rawDebug(abs, difference) + t.ok(difference < 0) t.end() }) From 51765a09ad96ae44abe9c54d3dcb3cb3d33f61dc Mon Sep 17 00:00:00 2001 From: "went.out" Date: Thu, 25 Apr 2019 12:22:25 +0300 Subject: [PATCH 8/8] reverting changes for the test --- test/cmd-collect.test.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/cmd-collect.test.js b/test/cmd-collect.test.js index 2d27790d..64097444 100644 --- a/test/cmd-collect.test.js +++ b/test/cmd-collect.test.js @@ -88,14 +88,10 @@ test('cmd - collect - data files have content', function (t) { if (err) return t.ifError(err) // expect time seperation to be 10ms, allow 20ms error - const allow = 20 const sampleTimes = output.processStat.map((stat) => stat.timestamp) const timeSeperation = summary(diff(sampleTimes)).mean() t.ok(sampleTimes.length > 0, 'data is outputted') - const abs = Math.abs(timeSeperation - 10) - const difference = abs - allow - process._rawDebug(abs, difference) - t.ok(difference < 0) + t.ok(Math.abs(timeSeperation - 10) < 20) t.end() })