From 5d429f2e98e782d584486f4b8b675a5a26d871b6 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.
---
 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 +++++-
 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 ++
 13 files changed, 130 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 d5632532f80a6f..9b45262beae077 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -592,6 +592,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
@@ -1136,6 +1145,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 4a33b3b9cbefe2..355632ef9e9a1a 100644
--- a/doc/api/process.md
+++ b/doc/api/process.md
@@ -1795,6 +1795,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 7762424ec5f7b6..6fa8d92a9bf4b4 100644
--- a/doc/api/report.md
+++ b/doc/api/report.md
@@ -419,6 +419,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 9d75596c60ec6e..149902d3195543 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -281,6 +281,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/src/node_options.cc b/src/node_options.cc
index a555ac5cb3e0ee..1eb2f586360044 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -577,6 +577,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 a4174767c9b2ba..66635122cbb558 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -187,6 +187,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');