Skip to content

Commit

Permalink
src: allow setting a dir for all diagnostic output
Browse files Browse the repository at this point in the history
Add a flag that allows for the setting of a directory where all
diagnostic output will be written to.
e.g. --redirect-warnings

Refs: #33010 (comment)

PR-URL: #33584
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
AshCripps authored and MylesBorins committed Jul 27, 2020
1 parent 74da2c4 commit 5c94358
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 2 deletions.
21 changes: 21 additions & 0 deletions doc/api/cli.md
Expand Up @@ -108,6 +108,9 @@ added: v12.0.0
Specify the directory where the CPU profiles generated by `--cpu-prof` will
be placed.

The default value is controlled by the
[--diagnostic-dir](#cli_diagnostic_dir_directory) command line option.

### `--cpu-prof-interval`
<!-- YAML
added: v12.2.0
Expand All @@ -127,6 +130,16 @@ added: v12.0.0
Specify the file name of the CPU profile generated by `--cpu-prof`.

### `--diagnostic-dir=directory`

Set the directory to which all diagnostic output files will be written to.
Defaults to current working directory.

Affects the default output directory of:
* [--cpu-prof-dir](#cli_cpu_prof_dir)
* [--heap-prof-dir](#cli_heap_prof_dir)
* [--redirect-warnings](#cli_redirect_warnings_file)

### `--disable-proto=mode`
<!--YAML
added: v13.12.0
Expand Down Expand Up @@ -346,6 +359,9 @@ added: v12.4.0
Specify the directory where the heap profiles generated by `--heap-prof` will
be placed.

The default value is controlled by the
[--diagnostic-dir](#cli_diagnostic_dir_directory) command line option.

### `--heap-prof-interval`
<!-- YAML
added: v12.4.0
Expand Down Expand Up @@ -626,6 +642,10 @@ file will be created if it does not exist, and will be appended to if it does.
If an error occurs while attempting to write the warning to the file, the
warning will be written to stderr instead.

The `file` name may be an absolute path. If it is not, the default directory it
will be written to is controlled by the
[--diagnostic-dir](#cli_diagnostic_dir_directory) command line option.

### `--report-compact`
<!-- YAML
added: v13.12.0
Expand Down Expand Up @@ -1189,6 +1209,7 @@ node --require "./a.js" --require "./b.js"

Node.js options that are allowed are:
<!-- node-options-node start -->
* `--diagnostic-dir`
* `--disable-proto`
* `--enable-fips`
* `--enable-source-maps`
Expand Down
21 changes: 21 additions & 0 deletions doc/node.1
Expand Up @@ -89,6 +89,9 @@ with a generated file name.
The directory where the CPU profiles generated by
.Fl -cpu-prof
will be placed.
The default value is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -cpu-prof-interval
The sampling interval in microseconds for the CPU profiles generated by
Expand All @@ -100,6 +103,17 @@ The default is
File name of the V8 CPU profile generated with
.Fl -cpu-prof
.
.It Fl -diagnostic-dir
Set the directory for all diagnostic output files.
Default is current working directory.
Set the directory to which all diagnostic output files will be written to.
Defaults to current working directory.
.
Affects the default output directory of:
.Fl -cpu-prof-dir .
.Fl -heap-prof-dir .
.Fl -redirect-warnings .
.
.It Fl -disable-proto Ns = Ns Ar mode
Disable the `Object.prototype.__proto__` property. If
.Ar mode
Expand Down Expand Up @@ -181,6 +195,9 @@ with a generated file name.
The directory where the heap profiles generated by
.Fl -heap-prof
will be placed.
The default value is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -heap-prof-interval
The average sampling interval in bytes for the heap profiles generated by
Expand Down Expand Up @@ -298,6 +315,10 @@ in a compact format, single-line JSON.
Location at which the
.Sy diagnostic report
will be generated.
The `file` name may be an absolute path. If it is not, the default directory it will
be written to is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -report-filename
Name of the file to which the
Expand Down
12 changes: 10 additions & 2 deletions lib/internal/process/warning.js
Expand Up @@ -12,13 +12,21 @@ const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
let fs;
let fd;
let warningFile;
let options;

function lazyOption() {
// This will load `warningFile` only once. If the flag is not set,
// `warningFile` will be set to an empty string.
if (warningFile === undefined) {
warningFile = require('internal/options')
.getOptionValue('--redirect-warnings');
options = require('internal/options');
if (options.getOptionValue('--diagnostic-dir') !== '') {
warningFile = options.getOptionValue('--diagnostic-dir');
}
if (options.getOptionValue('--redirect-warnings') !== '') {
warningFile = options.getOptionValue('--redirect-warnings');
} else {
warningFile = '';
}
}
return warningFile;
}
Expand Down
14 changes: 14 additions & 0 deletions src/node_options.cc
Expand Up @@ -146,6 +146,10 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
}
}

if (cpu_prof && cpu_prof_dir.empty() && !diagnostic_dir.empty()) {
cpu_prof_dir = diagnostic_dir;
}

if (!heap_prof) {
if (!heap_prof_name.empty()) {
errors->push_back("--heap-prof-name must be used with --heap-prof");
Expand All @@ -159,6 +163,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
errors->push_back("--heap-prof-interval must be used with --heap-prof");
}
}

if (heap_prof && heap_prof_dir.empty() && !diagnostic_dir.empty()) {
heap_prof_dir = diagnostic_dir;
}

debug_options_.CheckOptions(errors);
#endif // HAVE_INSPECTOR
}
Expand Down Expand Up @@ -272,6 +281,11 @@ DebugOptionsParser::DebugOptionsParser() {
}

EnvironmentOptionsParser::EnvironmentOptionsParser() {
AddOption("--diagnostic-dir",
"set dir for all output files"
" (default: current working directory)",
&EnvironmentOptions::diagnostic_dir,
kAllowedInEnvironment);
AddOption("--enable-source-maps",
"experimental Source Map V3 support",
&EnvironmentOptions::enable_source_maps,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Expand Up @@ -138,6 +138,7 @@ class EnvironmentOptions : public Options {
bool heap_prof = false;
#endif // HAVE_INSPECTOR
std::string redirect_warnings;
std::string diagnostic_dir;
bool test_udp_no_try_send = false;
bool throw_deprecation = false;
bool trace_atomics_wait = false;
Expand Down
76 changes: 76 additions & 0 deletions test/sequential/test-diagnostic-dir-cpu-prof.js
@@ -0,0 +1,76 @@
'use strict';

// This test is to ensure that --diagnostic-dir does not change the directory
// for --cpu-prof when --cpu-prof-dir is specified

const common = require('../common');
const fixtures = require('../common/fixtures');
common.skipIfInspectorDisabled();

const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');

const tmpdir = require('../common/tmpdir');
const {
getCpuProfiles,
kCpuProfInterval,
env,
verifyFrames
} = require('../common/cpu-prof');

// Test --diagnostic-dir changes the default for --cpu-prof

{
tmpdir.refresh();
const dir = path.join(tmpdir.path, 'prof');
const output = spawnSync(process.execPath, [
'--cpu-prof',
'--cpu-prof-interval',
kCpuProfInterval,
'--diagnostic-dir',
dir,
fixtures.path('workload', 'fibonacci.js'),
], {
cwd: tmpdir.path,
env
});
if (output.status !== 0) {
console.log(output.stderr.toString());
}
assert.strictEqual(output.status, 0);
assert(fs.existsSync(dir));
const profiles = getCpuProfiles(dir);
assert.strictEqual(profiles.length, 1);
verifyFrames(output, profiles[0], 'fibonacci.js');
}

// Test --cpu-prof-dir overwrites --diagnostic-dir

{
tmpdir.refresh();
const dir = path.join(tmpdir.path, 'diag');
const dir2 = path.join(tmpdir.path, 'prof');
const output = spawnSync(process.execPath, [
'--cpu-prof',
'--cpu-prof-interval',
kCpuProfInterval,
'--diagnostic-dir',
dir,
'--cpu-prof-dir',
dir2,
fixtures.path('workload', 'fibonacci.js'),
], {
cwd: tmpdir.path,
env
});
if (output.status !== 0) {
console.log(output.stderr.toString());
}
assert.strictEqual(output.status, 0);
assert(fs.existsSync(dir2));
const profiles = getCpuProfiles(dir2);
assert.strictEqual(profiles.length, 1);
verifyFrames(output, profiles[0], 'fibonacci.js');
}
117 changes: 117 additions & 0 deletions test/sequential/test-diagnostic-dir-heap-prof.js
@@ -0,0 +1,117 @@
'use strict';

// This test is to ensure that --diagnostic-dir does not change the directory
// for --cpu-prof when --cpu-prof-dir is specified

const common = require('../common');
const fixtures = require('../common/fixtures');
common.skipIfInspectorDisabled();

const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');

const tmpdir = require('../common/tmpdir');

function findFirstFrameInNode(root, func) {
const first = root.children.find(
(child) => child.callFrame.functionName === func
);
if (first) {
return first;
}
for (const child of root.children) {
const first = findFirstFrameInNode(child, func);
if (first) {
return first;
}
}
return undefined;
}

function findFirstFrame(file, func) {
const data = fs.readFileSync(file, 'utf8');
const profile = JSON.parse(data);
const first = findFirstFrameInNode(profile.head, func);
return { frame: first, roots: profile.head.children };
}

function verifyFrames(output, file, func) {
const { frame, roots } = findFirstFrame(file, func);
if (!frame) {
// Show native debug output and the profile for debugging.
console.log(output.stderr.toString());
console.log(roots);
}
assert.notDeepStrictEqual(frame, undefined);
}

const kHeapProfInterval = 128;
const TEST_ALLOCATION = kHeapProfInterval * 2;

const env = {
...process.env,
TEST_ALLOCATION,
NODE_DEBUG_NATIVE: 'INSPECTOR_PROFILER'
};

function getHeapProfiles(dir) {
const list = fs.readdirSync(dir);
return list
.filter((file) => file.endsWith('.heapprofile'))
.map((file) => path.join(dir, file));
}

// Test --diagnostic-dir changes the default for --cpu-prof
{
tmpdir.refresh();
const dir = path.join(tmpdir.path, 'prof');
const output = spawnSync(process.execPath, [
'--heap-prof',
'--diagnostic-dir',
dir,
'--heap-prof-interval',
kHeapProfInterval,
fixtures.path('workload', 'allocation.js'),
], {
cwd: tmpdir.path,
env
});
if (output.status !== 0) {
console.log(output.stderr.toString());
}
assert.strictEqual(output.status, 0);
assert(fs.existsSync(dir));
const profiles = getHeapProfiles(dir);
assert.strictEqual(profiles.length, 1);
verifyFrames(output, profiles[0], 'runAllocation');
}

// Test --heap-prof-dir overwrites --diagnostic-dir
{
tmpdir.refresh();
const dir = path.join(tmpdir.path, 'diag');
const dir2 = path.join(tmpdir.path, 'prof');
const output = spawnSync(process.execPath, [
'--heap-prof',
'--heap-prof-interval',
kHeapProfInterval,
'--diagnostic-dir',
dir,
'--heap-prof-dir',
dir2,
fixtures.path('workload', 'allocation.js'),
], {
cwd: tmpdir.path,
env
});
if (output.status !== 0) {
console.log(output.stderr.toString());
}
assert.strictEqual(output.status, 0);
assert(fs.existsSync(dir2));
const profiles = getHeapProfiles(dir2);
assert.strictEqual(profiles.length, 1);
verifyFrames(output, profiles[0], 'runAllocation');
}

0 comments on commit 5c94358

Please sign in to comment.