From 4ec25b4865e497b0e93974b3af6584008f279e5f Mon Sep 17 00:00:00 2001
From: Sam Roberts <vieuxtech@gmail.com>
Date: Fri, 13 Mar 2020 09:45:39 -0700
Subject: [PATCH] src,cli: support compact (one-line) JSON reports

Multi-line JSON is more human readable, but harder for log aggregators
to consume, they usually require a log message per line, particularly
for JSON. Compact output will be consumable by aggregators such as EFK
(Elastic Search-Fluentd-Kibana), LogDNA, DataDog, etc.

PR-URL: https://github.com/nodejs/node/pull/32254
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 doc/api/cli.md                                | 10 ++++
 doc/api/process.md                            | 15 ++++++
 doc/api/report.md                             |  4 ++
 doc/node.1                                    |  5 ++
 lib/internal/process/report.js                | 13 +++++-
 lib/internal/validators.js                    | 28 +++++++++++
 src/node_options.cc                           |  4 ++
 src/node_options.h                            |  1 +
 src/node_report.cc                            | 13 ++++--
 src/node_report.h                             | 46 ++++++++++++++-----
 src/node_report_module.cc                     | 14 ++++++
 test/common/report.js                         |  7 ++-
 test/report/test-report-config.js             | 13 +++++-
 .../test-report-uncaught-exception-compat.js  |  5 ++
 14 files changed, 158 insertions(+), 20 deletions(-)
 create mode 100644 test/report/test-report-uncaught-exception-compat.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index c050c177cfe06d..aedc223f9851fb 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -615,6 +615,15 @@ 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.
 
+### `--report-compact`
+<!-- YAML
+added: REPLACEME
+-->
+
+Write reports in a compact format, single-line JSON, more easily consumable
+by log processing systems than the default multi-line format designed for
+human consumption.
+
 ### `--report-directory=directory`
 <!-- YAML
 added: v11.8.0
@@ -1160,6 +1169,7 @@ Node.js options that are allowed are:
 * `--preserve-symlinks`
 * `--prof-process`
 * `--redirect-warnings`
+* `--report-compact`
 * `--report-directory`
 * `--report-filename`
 * `--report-on-fatalerror`
diff --git a/doc/api/process.md b/doc/api/process.md
index cdf31a3d52d60a..7df2401539719b 100644
--- a/doc/api/process.md
+++ b/doc/api/process.md
@@ -1767,6 +1767,21 @@ changes:
 reports for the current process. Additional documentation is available in the
 [report documentation][].
 
+### `process.report.compact`
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean}
+
+Write reports in a compact format, single-line JSON, more easily consumable
+by log processing systems than the default multi-line format designed for
+human consumption.
+
+```js
+console.log(`Reports are compact? ${process.report.compact}`);
+```
+
 ### `process.report.directory`
 <!-- YAML
 added: v11.12.0
diff --git a/doc/api/report.md b/doc/api/report.md
index 7071c30b039e70..f3db531780a2bf 100644
--- a/doc/api/report.md
+++ b/doc/api/report.md
@@ -420,6 +420,10 @@ that leads to termination of the application. Useful to inspect various
 diagnostic data elements such as heap, stack, event loop state, resource
 consumption etc. to reason about the fatal error.
 
+* `--report-compact` Write reports in a compact format, single-line JSON, more
+easily consumable by log processing systems than the default multi-line format
+designed for human consumption.
+
 * `--report-directory` Location at which the report will be
 generated.
 
diff --git a/doc/node.1 b/doc/node.1
index ca07cdef1c5a57..c127654748302d 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -290,6 +290,11 @@ Write process warnings to the given
 .Ar file
 instead of printing to stderr.
 .
+.It Fl -report-compact
+Write
+.Sy diagnostic reports
+in a compact format, single-line JSON.
+.
 .It Fl -report-directory
 Location at which the
 .Sy diagnostic report
diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js
index 407cc12cb04f76..72a8b0c1ebe337 100644
--- a/lib/internal/process/report.js
+++ b/lib/internal/process/report.js
@@ -3,7 +3,11 @@ const {
   ERR_INVALID_ARG_TYPE,
   ERR_SYNTHETIC
 } = require('internal/errors').codes;
-const { validateSignalName, validateString } = require('internal/validators');
+const {
+  validateSignalName,
+  validateString,
+  validateBoolean,
+} = require('internal/validators');
 const nr = internalBinding('report');
 const {
   JSONParse,
@@ -45,6 +49,13 @@ const report = {
     validateString(name, 'filename');
     nr.setFilename(name);
   },
+  get compact() {
+    return nr.getCompact();
+  },
+  set compact(b) {
+    validateBoolean(b, 'compact');
+    nr.setCompact(b);
+  },
   get signal() {
     return nr.getSignal();
   },
diff --git a/lib/internal/validators.js b/lib/internal/validators.js
index 5e23e72ed5128e..da238cd8e6f14b 100644
--- a/lib/internal/validators.js
+++ b/lib/internal/validators.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ArrayIsArray,
   NumberIsInteger,
   NumberMAX_SAFE_INTEGER,
   NumberMIN_SAFE_INTEGER,
@@ -123,6 +124,30 @@ function validateNumber(value, name) {
     throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
 }
 
+function validateBoolean(value, name) {
+  if (typeof value !== 'boolean')
+    throw new ERR_INVALID_ARG_TYPE(name, 'boolean', value);
+}
+
+const validateObject = hideStackFrames(
+  (value, name, { nullable = false } = {}) => {
+    if ((!nullable && value === null) ||
+        ArrayIsArray(value) ||
+        typeof value !== 'object') {
+      throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+    }
+  });
+
+const validateArray = hideStackFrames((value, name, { minLength = 0 } = {}) => {
+  if (!ArrayIsArray(value)) {
+    throw new ERR_INVALID_ARG_TYPE(name, 'Array', value);
+  }
+  if (value.length < minLength) {
+    const reason = `must be longer than ${minLength}`;
+    throw new ERR_INVALID_ARG_VALUE(name, value, reason);
+  }
+});
+
 function validateSignalName(signal, name = 'signal') {
   if (typeof signal !== 'string')
     throw new ERR_INVALID_ARG_TYPE(name, 'string', signal);
@@ -162,10 +187,13 @@ module.exports = {
   isInt32,
   isUint32,
   parseMode,
+  validateArray,
+  validateBoolean,
   validateBuffer,
   validateInt32,
   validateInteger,
   validateNumber,
+  validateObject,
   validatePort,
   validateSignalName,
   validateString,
diff --git a/src/node_options.cc b/src/node_options.cc
index f2c0024abc6670..87bb407101fa50 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -617,6 +617,10 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
             "generate diagnostic report on uncaught exceptions",
             &PerIsolateOptions::report_uncaught_exception,
             kAllowedInEnvironment);
+  AddOption("--report-compact",
+            "output compact single-line JSON",
+            &PerIsolateOptions::report_compact,
+            kAllowedInEnvironment);
   AddOption("--report-on-signal",
             "generate diagnostic report upon receiving signals",
             &PerIsolateOptions::report_on_signal,
diff --git a/src/node_options.h b/src/node_options.h
index 6cb5245492d733..5996e529aa3e72 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -188,6 +188,7 @@ class PerIsolateOptions : public Options {
   bool report_uncaught_exception = false;
   bool report_on_signal = false;
   bool report_on_fatalerror = false;
+  bool report_compact = false;
   std::string report_signal = "SIGUSR2";
   std::string report_filename;
   std::string report_directory;
diff --git a/src/node_report.cc b/src/node_report.cc
index c29f866f4a8dad..bb703594c9a237 100644
--- a/src/node_report.cc
+++ b/src/node_report.cc
@@ -52,7 +52,8 @@ static void WriteNodeReport(Isolate* isolate,
                             const char* trigger,
                             const std::string& filename,
                             std::ostream& out,
-                            Local<String> stackstr);
+                            Local<String> stackstr,
+                            bool compact);
 static void PrintVersionInformation(JSONWriter* writer);
 static void PrintJavaScriptStack(JSONWriter* writer,
                                  Isolate* isolate,
@@ -126,8 +127,9 @@ std::string TriggerNodeReport(Isolate* isolate,
     std::cerr << "\nWriting Node.js report to file: " << filename;
   }
 
+  bool compact = env != nullptr ? options->report_compact : true;
   WriteNodeReport(isolate, env, message, trigger, filename, *outstream,
-                  stackstr);
+                  stackstr, compact);
 
   // Do not close stdout/stderr, only close files we opened.
   if (outfile.is_open()) {
@@ -145,7 +147,7 @@ void GetNodeReport(Isolate* isolate,
                    const char* trigger,
                    Local<String> stackstr,
                    std::ostream& out) {
-  WriteNodeReport(isolate, env, message, trigger, "", out, stackstr);
+  WriteNodeReport(isolate, env, message, trigger, "", out, stackstr, false);
 }
 
 // Internal function to coordinate and write the various
@@ -156,7 +158,8 @@ static void WriteNodeReport(Isolate* isolate,
                             const char* trigger,
                             const std::string& filename,
                             std::ostream& out,
-                            Local<String> stackstr) {
+                            Local<String> stackstr,
+                            bool compact) {
   // Obtain the current time and the pid.
   TIME_TYPE tm_struct;
   DiagnosticFilename::LocalTime(&tm_struct);
@@ -169,7 +172,7 @@ static void WriteNodeReport(Isolate* isolate,
   // File stream opened OK, now start printing the report content:
   // the title and header information (event, filename, timestamp and pid)
 
-  JSONWriter writer(out);
+  JSONWriter writer(out, compact);
   writer.json_start();
   writer.json_objectstart("header");
   writer.json_keyvalue("reportVersion", NODE_REPORT_VERSION);
diff --git a/src/node_report.h b/src/node_report.h
index 46b69b9681db51..808e302223e5f8 100644
--- a/src/node_report.h
+++ b/src/node_report.h
@@ -65,17 +65,29 @@ extern double prog_start_time;
 // JSON compiler definitions.
 class JSONWriter {
  public:
-  explicit JSONWriter(std::ostream& out) : out_(out) {}
+  JSONWriter(std::ostream& out, bool compact)
+    : out_(out), compact_(compact) {}
 
+ private:
   inline void indent() { indent_ += 2; }
   inline void deindent() { indent_ -= 2; }
   inline void advance() {
+    if (compact_) return;
     for (int i = 0; i < indent_; i++) out_ << ' ';
   }
+  inline void write_one_space() {
+    if (compact_) return;
+    out_ << ' ';
+  }
+  inline void write_new_line() {
+    if (compact_) return;
+    out_ << '\n';
+  }
 
+ public:
   inline void json_start() {
     if (state_ == kAfterValue) out_ << ',';
-    out_ << '\n';
+    write_new_line();
     advance();
     out_ << '{';
     indent();
@@ -83,7 +95,7 @@ class JSONWriter {
   }
 
   inline void json_end() {
-    out_ << '\n';
+    write_new_line();
     deindent();
     advance();
     out_ << '}';
@@ -92,10 +104,12 @@ class JSONWriter {
   template <typename T>
   inline void json_objectstart(T key) {
     if (state_ == kAfterValue) out_ << ',';
-    out_ << '\n';
+    write_new_line();
     advance();
     write_string(key);
-    out_ << ": {";
+    out_ << ':';
+    write_one_space();
+    out_ << '{';
     indent();
     state_ = kObjectStart;
   }
@@ -103,23 +117,29 @@ class JSONWriter {
   template <typename T>
   inline void json_arraystart(T key) {
     if (state_ == kAfterValue) out_ << ',';
-    out_ << '\n';
+    write_new_line();
     advance();
     write_string(key);
-    out_ << ": [";
+    out_ << ':';
+    write_one_space();
+    out_ << '[';
     indent();
     state_ = kObjectStart;
   }
   inline void json_objectend() {
-    out_ << '\n';
+    write_new_line();
     deindent();
     advance();
     out_ << '}';
+    if (indent_ == 0) {
+      // Top-level object is complete, so end the line.
+      out_ << '\n';
+    }
     state_ = kAfterValue;
   }
 
   inline void json_arrayend() {
-    out_ << '\n';
+    write_new_line();
     deindent();
     advance();
     out_ << ']';
@@ -128,10 +148,11 @@ class JSONWriter {
   template <typename T, typename U>
   inline void json_keyvalue(const T& key, const U& value) {
     if (state_ == kAfterValue) out_ << ',';
-    out_ << '\n';
+    write_new_line();
     advance();
     write_string(key);
-    out_ << ": ";
+    out_ << ':';
+    write_one_space();
     write_value(value);
     state_ = kAfterValue;
   }
@@ -139,7 +160,7 @@ class JSONWriter {
   template <typename U>
   inline void json_element(const U& value) {
     if (state_ == kAfterValue) out_ << ',';
-    out_ << '\n';
+    write_new_line();
     advance();
     write_value(value);
     state_ = kAfterValue;
@@ -177,6 +198,7 @@ class JSONWriter {
 
   enum JSONState { kObjectStart, kAfterValue };
   std::ostream& out_;
+  bool compact_;
   int indent_ = 0;
   int state_ = kObjectStart;
 };
diff --git a/src/node_report_module.cc b/src/node_report_module.cc
index f35d23a06e3232..e06fe52514ef7f 100644
--- a/src/node_report_module.cc
+++ b/src/node_report_module.cc
@@ -68,6 +68,18 @@ void GetReport(const FunctionCallbackInfo<Value>& info) {
                                 .ToLocalChecked());
 }
 
+static void GetCompact(const FunctionCallbackInfo<Value>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  info.GetReturnValue().Set(env->isolate_data()->options()->report_compact);
+}
+
+static void SetCompact(const FunctionCallbackInfo<Value>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  Isolate* isolate = env->isolate();
+  bool compact = info[0]->ToBoolean(isolate)->Value();
+  env->isolate_data()->options()->report_compact = compact;
+}
+
 static void GetDirectory(const FunctionCallbackInfo<Value>& info) {
   Environment* env = Environment::GetCurrent(info);
   std::string directory = env->isolate_data()->options()->report_directory;
@@ -161,6 +173,8 @@ static void Initialize(Local<Object> exports,
 
   env->SetMethod(exports, "writeReport", WriteReport);
   env->SetMethod(exports, "getReport", GetReport);
+  env->SetMethod(exports, "getCompact", GetCompact);
+  env->SetMethod(exports, "setCompact", SetCompact);
   env->SetMethod(exports, "getDirectory", GetDirectory);
   env->SetMethod(exports, "setDirectory", SetDirectory);
   env->SetMethod(exports, "getFilename", GetFilename);
diff --git a/test/common/report.js b/test/common/report.js
index c117d7b76459bc..3b6b46eeec0f8d 100644
--- a/test/common/report.js
+++ b/test/common/report.js
@@ -25,7 +25,12 @@ function findReports(pid, dir) {
 }
 
 function validate(filepath) {
-  validateContent(JSON.parse(fs.readFileSync(filepath, 'utf8')));
+  const report = fs.readFileSync(filepath, 'utf8');
+  if (process.report.compact) {
+    const end = report.indexOf('\n');
+    assert.strictEqual(end, report.length - 1);
+  }
+  validateContent(JSON.parse(report));
 }
 
 function validateContent(report) {
diff --git a/test/report/test-report-config.js b/test/report/test-report-config.js
index f179925440ab7b..a8c0f70691e63f 100644
--- a/test/report/test-report-config.js
+++ b/test/report/test-report-config.js
@@ -1,4 +1,4 @@
-// Flags: --report-on-fatalerror --report-on-signal --report-uncaught-exception
+// Flags: --report-on-fatalerror --report-on-signal --report-uncaught-exception --report-compact
 'use strict';
 const common = require('../common');
 const assert = require('assert');
@@ -55,6 +55,17 @@ assert.throws(() => {
 }, { code: 'ERR_INVALID_ARG_TYPE' });
 assert.strictEqual(process.report.reportOnSignal, true);
 
+// Verify that process.report.reportCompact behaves properly.
+assert.strictEqual(process.report.compact, true);
+process.report.compact = false;
+assert.strictEqual(process.report.compact, false);
+process.report.compact = true;
+assert.strictEqual(process.report.compact, true);
+assert.throws(() => {
+  process.report.compact = {};
+}, { code: 'ERR_INVALID_ARG_TYPE' });
+assert.strictEqual(process.report.compact, true);
+
 if (!common.isWindows) {
   // Verify that process.report.signal behaves properly.
   assert.strictEqual(process.report.signal, 'SIGUSR2');
diff --git a/test/report/test-report-uncaught-exception-compat.js b/test/report/test-report-uncaught-exception-compat.js
new file mode 100644
index 00000000000000..9fe690595e89a0
--- /dev/null
+++ b/test/report/test-report-uncaught-exception-compat.js
@@ -0,0 +1,5 @@
+// Flags: --experimental-report --report-uncaught-exception --report-compact
+'use strict';
+// Test producing a compact report on uncaught exception.
+require('../common');
+require('./test-report-uncaught-exception.js');