Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get analysis result out of tool.analyze() #273

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
13 changes: 11 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const doctor = new ClinicDoctor()
doctor.collect(['node', './path-to-script.js'], function (err, filepath) {
if (err) throw err

doctor.visualize(filepath, filepath + '.html', function (err) {
const analysis = doctor.analyze(filepath)
doctor.visualize(analysis, filepath + '.html', function (err) {
if (err) throw err
});
})
Expand Down Expand Up @@ -68,11 +69,19 @@ directory will be the value in the callback.
stdout, stderr, and stdin will be relayed to the calling process. As will the
`SIGINT` event.

#### `doctor.visualize(dataFilename, outputFilename, callback)`
#### `doctor.analyze(dataFilename)`

Will consume the datafile specified by `dataFilename`, this datafile will be
produced by the sampler using `doctor.collect`.

`doctor.analyze` will then return an object that can be passed to
`doctor.visualize()` to generate a visualization.

#### `doctor.visualize(analysis, outputFilename, callback)`

Will consume the analysis specified by `analysis`, which can be produced by
calling `doctor.analyze()`.

`doctor.visualize` will then output a standalone HTML file to `outputFilename`.
When completed the `callback` will be called with no extra arguments, except a
possible error.
Expand Down
2 changes: 1 addition & 1 deletion debug/visualize-mod.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
const tool = new Tool({ debug: true })

tool.visualize(
file,
tool.analyze(file),
file + '.html',
function (err) {
if (err) {
Expand Down
51 changes: 40 additions & 11 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,7 @@ class ClinicDoctor extends events.EventEmitter {
})
}

visualize (dataDirname, outputFilename, callback) {
const fakeDataPath = path.join(__dirname, 'visualizer', 'data.json')
const stylePath = path.join(__dirname, 'visualizer', 'style.css')
const scriptPath = path.join(__dirname, 'visualizer', 'main.js')
const logoPath = path.join(__dirname, 'visualizer', 'app-logo.svg')
const nearFormLogoPath = path.join(__dirname, 'visualizer', 'nearform-logo.svg')
const clinicFaviconPath = path.join(__dirname, 'visualizer', 'clinic-favicon.png.b64')

// Load data
analyze (dataDirname) {
const paths = getLoggingPaths({ path: dataDirname })

const systemInfoReader = pumpify.obj(
Expand All @@ -159,13 +151,46 @@ class ClinicDoctor extends events.EventEmitter {
new ProcessStatDecoder()
)

// create analysis
const analysis = new Analysis(systemInfoReader, traceEventReader, processStatReader)

return {
traceEventReader,
processStatReader,
analysis
}
}

visualize (analysisData, outputFilename, callback) {
const fakeDataPath = path.join(__dirname, 'visualizer', 'data.json')
const stylePath = path.join(__dirname, 'visualizer', 'style.css')
const scriptPath = path.join(__dirname, 'visualizer', 'main.js')
const logoPath = path.join(__dirname, 'visualizer', 'app-logo.svg')
const nearFormLogoPath = path.join(__dirname, 'visualizer', 'nearform-logo.svg')
const clinicFaviconPath = path.join(__dirname, 'visualizer', 'clinic-favicon.png.b64')

// back compat
if (typeof analysisData === 'string') {
analysisData = this.analyze(analysisData)
}

// Load data
const {
traceEventReader,
processStatReader,
analysis
} = analysisData

let result = null

// create analysis
const analysisStringified = pumpify(
new Analysis(systemInfoReader, traceEventReader, processStatReader),
analysis,
new stream.Transform({
readableObjectMode: false,
writableObjectMode: true,
transform (data, encoding, callback) {
result = data
callback(null, JSON.stringify(data))
}
})
Expand Down Expand Up @@ -274,7 +299,11 @@ class ClinicDoctor extends events.EventEmitter {
fs.createWriteStream(outputFilename),
function (err) {
clearInterval(checkHeapInterval)
callback(err)
if (err) {
callback(err)
} else {
callback(null, result)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having visualize output a structured object is a bit odd. If anything, it should output the file stream. A better separation of logic would be collect() -> analyze() -> visualize(). Where visualize() takes the analyze() output. I do see the incompatibility, but maybe analyze() should output streams?

I don't have any strong opinions. A valid alternative is also to just force the API consumer to call both methods if they want both a structured object and a file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think analyze()visualize() would be ideal, but I didn't wanna break the API 😛 I did it this way so the CLI could avoid running the analysis twice. I agree with your points tho.

}
}
)
}
Expand Down
1 change: 1 addition & 0 deletions recommendations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,4 @@ class RenderRecommendations extends stream.Readable {
}

module.exports = RenderRecommendations
module.exports.recommendations = recommendations
162 changes: 162 additions & 0 deletions test/cmd-analyze.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
'use strict'
/* eslint-disable */

const fs = require('fs')
const v8 = require('v8')
const { test } = require('tap')
const pump = require('pump')
const async = require('async')
const rimraf = require('rimraf')
const mkdirp = require('mkdirp')
const semver = require('semver')
const startpoint = require('startpoint')
const ClinicDoctor = require('../index.js')
const getLoggingPaths = require('@nearform/clinic-common').getLoggingPaths('doctor')
const generateProcessStat = require('./generate-process-stat.js')
const generateTraceEvent = require('./generate-trace-event.js')

test('cmd - test analyze - data exists', function (t) {
const tool = new ClinicDoctor({ dest: './foo' })

function cleanup (err, dirname) {
t.ifError(err)

t.match(dirname, /^foo(\/|\\)[0-9]+\.clinic-doctor$/)

rimraf(dirname, function (err) {
t.ifError(err)
t.end()
})
}

const systemInfo = {
clock: {
hrtime: process.hrtime(),
unixtime: Date.now()
},
nodeVersions: process.versions
}

const badCPU = generateProcessStat({
cpu: [
// duplicate a bunch of times so the interval trimming code
// doesn't discard everything
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200,
200, 200, 15, 10, 190, 200, 5, 15, 190, 200
]
}, 0)

const goodMemoryGC = generateTraceEvent(
'-S....-S....-S....-S....-S....-S....-S....-S....-S....-S....' +
'-M....-M....-S....-M....-M....-S....-M....-M....-FFF.. CCC..'
)

const paths = getLoggingPaths({
identifier: 1234,
path: tool.path
})

mkdirp(paths['/'], ondir)

function ondir (err) {
if (err) return cleanup(err, paths['/'])
const ProcessStatEncoder = require('../format/process-stat-encoder.js')

async.parallel({
systeminfo (callback) {
fs.writeFile(paths['/systeminfo'], JSON.stringify(systemInfo), callback)
},
processstat (callback) {
pump(
startpoint(badCPU, { objectMode: true }),
new ProcessStatEncoder(),
fs.createWriteStream(paths['/processstat']),
callback)
},
traceevent (callback) {
fs.writeFile(paths['/traceevent'], JSON.stringify({ traceEvents: goodMemoryGC }), callback)
}
}, oncollected)
}

function oncollected (err) {
if (err) return cleanup(err, paths['/'])

tool.analyze(paths['/']).analysis
.on('error', function (err) { cleanup(err, paths['/']) })
.on('data', function (result) {
t.ok(result)
t.same(result.issueCategory, 'performance')
cleanup(null, paths['/'])
})
}
})

test('cmd - test analyze - memory exhausted', function (t) {
const tmp = process.memoryUsage
const HEAP_MAX = v8.getHeapStatistics().heap_size_limit

// Mock the used function to pretend the memory is exhausted.
process.memoryUsage = () => {
return {
rss: 0,
heapTotal: HEAP_MAX,
heapUsed: 0,
external: 0
}
}

const tool = new ClinicDoctor()

function cleanup (err, dirname) {
process.memoryUsage = tmp
t.ifError(err)

t.match(dirname, /^[0-9]+\.clinic-doctor$/)

rimraf(dirname, function (err) {
t.ifError(err)
t.end()
})
}

tool.on('warning', function (warning) {
t.equal(warning, 'Truncating input data due to memory constrains')
})
tool.on('truncate', function (undef) {
t.equal(undef, undefined)
})

tool.collect(
[process.execPath, '-e', 'setTimeout(() => {}, 400)'],
function (err, dirname) {
if (err) return cleanup(err, dirname)

tool.analyze(dirname).analysis
.on('error', function (err) { cleanup(err, dirname) })
.on('data', function (result) {
t.ok(result)
t.same(result.issueCategory, 'data')
cleanup(null, dirname)
})
}
)
})

test('cmd - test analyze - missing data', function (t) {
const tool = new ClinicDoctor({ debug: true })

tool.analyze('missing.clinic-doctor').analysis
.on('error', function (err) {
t.ok(err.message.includes('ENOENT: no such file or directory'))
t.end()
})
.on('data', function () {
t.fail('should error')
})
})