Skip to content

Commit

Permalink
report: add --report-exclude-network option
Browse files Browse the repository at this point in the history
New option `--report-exclude-network`, also available as
`report.excludeNetwork`, enables the user to exclude
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

PR-URL: #51645

Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
2 people authored and marco-ippolito committed May 2, 2024
1 parent fc9ba17 commit 29f09f0
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 10 deletions.
10 changes: 10 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,15 @@ Enables report to be generated when the process exits due to an uncaught
exception. Useful when inspecting the JavaScript stack in conjunction with
native stack and other runtime environment data.

### `--report-exclude-network`

<!-- YAML
added: REPLACEME
-->

Exclude `header.networkInterfaces` from the diagnostic report. By default
this is not set and the network interfaces are included.

### `-r`, `--require module`

<!-- YAML
Expand Down Expand Up @@ -2498,6 +2507,7 @@ Node.js options that are allowed are:
* `--redirect-warnings`
* `--report-compact`
* `--report-dir`, `--report-directory`
* `--report-exclude-network`
* `--report-filename`
* `--report-on-fatalerror`
* `--report-on-signal`
Expand Down
16 changes: 16 additions & 0 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
<!-- name=report -->

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/51645
description: Added `--report-exclude-network` option for excluding networking operations that can slow down report generation in some cases.
-->

Delivers a JSON-formatted diagnostic summary, written to a file.

The report is intended for development, test, and production use, to capture
Expand Down Expand Up @@ -452,6 +459,10 @@ meaning of `SIGUSR2` for the said purposes.
* `--report-signal` Sets or resets the signal for report generation
(not supported on Windows). Default signal is `SIGUSR2`.

* `--report-exclude-network` Exclude `header.networkInterfaces` from the
diagnostic report. By default this is not set and the network interfaces
are included.

A report can also be triggered via an API call from a JavaScript application:

```js
Expand Down Expand Up @@ -571,6 +582,8 @@ timestamp, PID, and sequence number.
written. URLs are not supported. Defaults to the current working directory of
the Node.js process.

`excludeNetwork` excludes `header.networkInterfaces` from the diagnostic report.

```js
// Trigger report only on uncaught exceptions.
process.report.reportOnFatalError = false;
Expand All @@ -587,6 +600,9 @@ process.report.reportOnFatalError = false;
process.report.reportOnUncaughtException = false;
process.report.reportOnSignal = true;
process.report.signal = 'SIGQUIT';

// Disable network interfaces reporting
process.report.excludeNetwork = true;
```

Configuration on module initialization is also available via
Expand Down
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 excludeNetwork() {
return nr.getExcludeNetwork();
},
set excludeNetwork(b) {
validateBoolean(b, 'excludeNetwork');
nr.setExcludeNetwork(b);
},
get signal() {
return nr.getSignal();
},
Expand Down
6 changes: 6 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"set default TLS maximum to TLSv1.3 (default: TLSv1.3)",
&EnvironmentOptions::tls_max_v1_3,
kAllowedInEnvvar);

AddOption("--report-exclude-network",
"exclude network interface diagnostics."
" (default: false)",
&EnvironmentOptions::report_exclude_network,
kAllowedInEnvvar);
}

PerIsolateOptionsParser::PerIsolateOptionsParser(
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ class EnvironmentOptions : public Options {

std::vector<std::string> user_argv;

bool report_exclude_network = false;

inline DebugOptions* get_debug_options() { return &debug_options_; }
inline const DebugOptions& debug_options() const { return debug_options_; }

Expand Down
40 changes: 30 additions & 10 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ 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 exclude_network = false);
static void PrintVersionInformation(JSONWriter* writer,
bool exclude_network = false);
static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<Value> error,
Expand Down Expand Up @@ -93,7 +95,8 @@ static void WriteNodeReport(Isolate* isolate,
const std::string& filename,
std::ostream& out,
Local<Value> error,
bool compact) {
bool compact,
bool exclude_network) {
// Obtain the current time and the pid.
TIME_TYPE tm_struct;
DiagnosticFilename::LocalTime(&tm_struct);
Expand Down Expand Up @@ -174,7 +177,7 @@ static void WriteNodeReport(Isolate* isolate,
}

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

if (isolate != nullptr) {
Expand Down Expand Up @@ -256,7 +259,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 exclude_network) {
std::ostringstream buf;
// Report Node version
buf << "v" << NODE_VERSION_STRING;
Expand Down Expand Up @@ -300,7 +303,7 @@ static void PrintVersionInformation(JSONWriter* writer) {
}

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

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

report::WriteNodeReport(
isolate, env, message, trigger, filename, *outstream, error, compact);
bool exclude_network = env != nullptr ? env->options()->report_exclude_network
: per_process::cli_options->per_isolate
->per_env->report_exclude_network;

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

// Do not close stdout/stderr, only close files we opened.
if (outfile.is_open()) {
Expand Down Expand Up @@ -969,8 +983,11 @@ void GetNodeReport(Isolate* isolate,
if (isolate != nullptr) {
env = Environment::GetCurrent(isolate);
}
bool exclude_network = env != nullptr ? env->options()->report_exclude_network
: per_process::cli_options->per_isolate
->per_env->report_exclude_network;
report::WriteNodeReport(
isolate, env, message, trigger, "", out, error, false);
isolate, env, message, trigger, "", out, error, false, exclude_network);
}

// External function to trigger a report, writing to a supplied stream.
Expand All @@ -983,8 +1000,11 @@ void GetNodeReport(Environment* env,
if (env != nullptr) {
isolate = env->isolate();
}
bool exclude_network = env != nullptr ? env->options()->report_exclude_network
: per_process::cli_options->per_isolate
->per_env->report_exclude_network;
report::WriteNodeReport(
isolate, env, message, trigger, "", out, error, false);
isolate, env, message, trigger, "", out, error, false, exclude_network);
}

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

static void GetExcludeNetwork(const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
info.GetReturnValue().Set(env->options()->report_exclude_network);
}

static void SetExcludeNetwork(const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
CHECK(info[0]->IsBoolean());
env->options()->report_exclude_network = info[0]->IsTrue();
}

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 +185,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, "getExcludeNetwork", GetExcludeNetwork);
SetMethod(context, exports, "setExcludeNetwork", SetExcludeNetwork);
SetMethod(context, exports, "getDirectory", GetDirectory);
SetMethod(context, exports, "setDirectory", SetDirectory);
SetMethod(context, exports, "getFilename", GetFilename);
Expand All @@ -200,6 +213,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetReport);
registry->Register(GetCompact);
registry->Register(SetCompact);
registry->Register(GetExcludeNetwork);
registry->Register(SetExcludeNetwork);
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.excludeNetwork behaves properly.
assert.strictEqual(process.report.excludeNetwork, false);
process.report.excludeNetwork = true;
assert.strictEqual(process.report.excludeNetwork, true);
process.report.excludeNetwork = false;
assert.strictEqual(process.report.excludeNetwork, false);
assert.throws(() => {
process.report.excludeNetwork = {};
}, { code: 'ERR_INVALID_ARG_TYPE' });
assert.strictEqual(process.report.excludeNetwork, false);

if (!common.isWindows) {
// Verify that process.report.signal behaves properly.
assert.strictEqual(process.report.signal, 'SIGUSR2');
Expand Down
41 changes: 41 additions & 0 deletions test/report/test-report-exclude-network.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';
require('../common');
const assert = require('node:assert');
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.strictEqual(reports.length, 1);
let report = fs.readFileSync(reports[0], { encoding: 'utf8' });
report = JSON.parse(report);
assert.strictEqual(report.header.networkInterfaces, undefined);
fs.unlinkSync(reports[0]);
}

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

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

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

const report = process.report.getReport();
assert.strictEqual(report.header.networkInterfaces, undefined);
});
});

0 comments on commit 29f09f0

Please sign in to comment.