Skip to content

Commit

Permalink
report: add --report-disable-network option
Browse files Browse the repository at this point in the history
New option `--report-disable-network`, also available as `report.disableNetwork`, enables the user to disable networking interfaces in their diagnostic report. On some systems, this can cause the report to take minutes to generate so this option can be used to optimize that.

fixes: #46060
  • Loading branch information
Ethan-Arrowood committed Feb 2, 2024
1 parent 68885d5 commit 8341898
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 9 deletions.
7 changes: 7 additions & 0 deletions lib/internal/process/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ const report = {
validateBoolean(b, 'compact');
nr.setCompact(b);
},
get disableNetwork() {
return nr.getDisableNetwork();
},
set disableNetwork(b) {
validateBoolean(b, 'disableNetwork');
nr.setDisableNetwork(b);
},
get signal() {
return nr.getSignal();
},
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,11 @@ PerProcessOptionsParser::PerProcessOptionsParser(
"generate diagnostic report on fatal (internal) errors",
&PerProcessOptions::report_on_fatalerror,
kAllowedInEnvvar);
AddOption("--report-disable-network",
"disable network interface diagnostics."
" (default: false)",
&PerProcessOptions::report_disable_network,
kAllowedInEnvvar);

#ifdef NODE_HAVE_I18N_SUPPORT
AddOption("--icu-data-dir",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ class PerProcessOptions : public Options {
bool report_compact = false;
std::string report_directory;
std::string report_filename;
bool report_disable_network = false;

// TODO(addaleax): Some of these could probably be per-Environment.
std::string use_largepages = "off";
Expand Down
37 changes: 28 additions & 9 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ static void WriteNodeReport(Isolate* isolate,
const std::string& filename,
std::ostream& out,
Local<Value> error,
bool compact);
static void PrintVersionInformation(JSONWriter* writer);
bool compact,
bool disable_network);
static void PrintVersionInformation(JSONWriter* writer, bool disable_network);
static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<Value> error,
Expand Down Expand Up @@ -93,7 +94,8 @@ static void WriteNodeReport(Isolate* isolate,
const std::string& filename,
std::ostream& out,
Local<Value> error,
bool compact) {
bool compact,
bool disable_network) {
// Obtain the current time and the pid.
TIME_TYPE tm_struct;
DiagnosticFilename::LocalTime(&tm_struct);
Expand Down Expand Up @@ -174,7 +176,7 @@ static void WriteNodeReport(Isolate* isolate,
}

// Report Node.js and OS version information
PrintVersionInformation(&writer);
PrintVersionInformation(&writer, disable_network);
writer.json_objectend();

if (isolate != nullptr) {
Expand Down Expand Up @@ -256,7 +258,7 @@ static void WriteNodeReport(Isolate* isolate,
}

// Report Node.js version, OS version and machine information.
static void PrintVersionInformation(JSONWriter* writer) {
static void PrintVersionInformation(JSONWriter* writer, bool disable_network) {
std::ostringstream buf;
// Report Node version
buf << "v" << NODE_VERSION_STRING;
Expand Down Expand Up @@ -300,7 +302,8 @@ static void PrintVersionInformation(JSONWriter* writer) {
}

PrintCpuInfo(writer);
PrintNetworkInterfaceInfo(writer);
if (!disable_network)
PrintNetworkInterfaceInfo(writer);

char host[UV_MAXHOSTNAMESIZE];
size_t host_size = sizeof(host);
Expand Down Expand Up @@ -917,8 +920,14 @@ std::string TriggerNodeReport(Isolate* isolate,
compact = per_process::cli_options->report_compact;
}

bool disable_network;
{
Mutex::ScopedLock lock(per_process::cli_options_mutex);
disable_network = per_process::cli_options->report_disable_network;
}

report::WriteNodeReport(
isolate, env, message, trigger, filename, *outstream, error, compact);
isolate, env, message, trigger, filename, *outstream, error, compact, disable_network);

// Do not close stdout/stderr, only close files we opened.
if (outfile.is_open()) {
Expand Down Expand Up @@ -969,8 +978,13 @@ void GetNodeReport(Isolate* isolate,
if (isolate != nullptr) {
env = Environment::GetCurrent(isolate);
}
bool disable_network;
{
Mutex::ScopedLock lock(per_process::cli_options_mutex);
disable_network = per_process::cli_options->report_disable_network;
}
report::WriteNodeReport(
isolate, env, message, trigger, "", out, error, false);
isolate, env, message, trigger, "", out, error, false, disable_network);
}

// External function to trigger a report, writing to a supplied stream.
Expand All @@ -983,8 +997,13 @@ void GetNodeReport(Environment* env,
if (env != nullptr) {
isolate = env->isolate();
}
bool disable_network;
{
Mutex::ScopedLock lock(per_process::cli_options_mutex);
disable_network = per_process::cli_options->report_disable_network;
}
report::WriteNodeReport(
isolate, env, message, trigger, "", out, error, false);
isolate, env, message, trigger, "", out, error, false, disable_network);
}

} // namespace node
17 changes: 17 additions & 0 deletions src/node_report_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ static void SetCompact(const FunctionCallbackInfo<Value>& info) {
per_process::cli_options->report_compact = compact;
}

static void GetDisableNetwork(const FunctionCallbackInfo<Value>& info) {
Mutex::ScopedLock lock(per_process::cli_options_mutex);
info.GetReturnValue().Set(per_process::cli_options->report_disable_network);
}

static void SetDisableNetwork(const FunctionCallbackInfo<Value>& info) {
Mutex::ScopedLock lock(per_process::cli_options_mutex);
Environment* env = Environment::GetCurrent(info);
Isolate* isolate = env->isolate();
bool disable_network = info[0]->ToBoolean(isolate)->Value();
per_process::cli_options->report_disable_network = disable_network;
}

static void GetDirectory(const FunctionCallbackInfo<Value>& info) {
Mutex::ScopedLock lock(per_process::cli_options_mutex);
Environment* env = Environment::GetCurrent(info);
Expand Down Expand Up @@ -174,6 +187,8 @@ static void Initialize(Local<Object> exports,
SetMethod(context, exports, "getReport", GetReport);
SetMethod(context, exports, "getCompact", GetCompact);
SetMethod(context, exports, "setCompact", SetCompact);
SetMethod(context, exports, "getDisableNetwork", GetDisableNetwork);
SetMethod(context, exports, "setDisableNetwork", SetDisableNetwork);
SetMethod(context, exports, "getDirectory", GetDirectory);
SetMethod(context, exports, "setDirectory", SetDirectory);
SetMethod(context, exports, "getFilename", GetFilename);
Expand All @@ -200,6 +215,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetReport);
registry->Register(GetCompact);
registry->Register(SetCompact);
registry->Register(GetDisableNetwork);
registry->Register(SetDisableNetwork);
registry->Register(GetDirectory);
registry->Register(SetDirectory);
registry->Register(GetFilename);
Expand Down
11 changes: 11 additions & 0 deletions test/report/test-report-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ assert.throws(() => {
}, { code: 'ERR_INVALID_ARG_TYPE' });
assert.strictEqual(process.report.compact, true);

// Verify that process.report.reportDisableNetwork behaves properly.
assert.strictEqual(process.report.disableNetwork, true);
process.report.disableNetwork = false;
assert.strictEqual(process.report.disableNetwork, false);
process.report.disableNetwork = true;
assert.strictEqual(process.report.disableNetwork, true);
assert.throws(() => {
process.report.disableNetwork = {};
}, { code: 'ERR_INVALID_ARG_TYPE' });
assert.strictEqual(process.report.disableNetwork, true);

if (!common.isWindows) {
// Verify that process.report.signal behaves properly.
assert.strictEqual(process.report.signal, 'SIGUSR2');
Expand Down
37 changes: 37 additions & 0 deletions test/report/test-report-disable-network.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';

Check failure on line 1 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Mandatory module "common" must be loaded

Check failure on line 1 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Mandatory module "common" must be loaded before any other modules
const assert = require('node:assert/strict');
const { spawnSync } = require('node:child_process');
const tmpdir = require('../common/tmpdir');
const { describe, it, before } = require('node:test');
const fs = require('node:fs');
const helper = require('../common/report');

function validate(pid) {
const reports = helper.findReports(pid, tmpdir.path);
assert.equal(reports.length, 1);

Check failure on line 11 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
let report = fs.readFileSync(reports[0], { encoding: 'utf8' });
report = JSON.parse(report);
assert.equal(report.header.networkInterfaces, undefined);

Check failure on line 14 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
fs.unlinkSync(reports[0]);
}

describe('report disable network option', () => {
before(() => {
tmpdir.refresh();
process.report.directory = tmpdir.path;
});

it('should be configurable with --report-disable-network', () => {
const args = ['--report-disable-network', '-e', 'process.report.writeReport()'];
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
assert.equal(child.status, 0);

Check failure on line 27 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
assert.equal(child.signal, null);

Check failure on line 28 in test/report/test-report-disable-network.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
validate(child.pid);
});

it('should be configurable with report.disableNetwork', () => {
process.report.disableNetwork = true;
process.report.writeReport();
validate(process.pid);
});
});

0 comments on commit 8341898

Please sign in to comment.