fix: log json in production (#1598)

Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>

src/apps/setup.ts

@@ -7,6 +7,7 @@ import { Probot } from "../probot";
 import { ManifestCreation } from "../manifest-creation";
 import { getLoggingMiddleware } from "../server/logging-middleware";
 import { ApplicationFunctionOptions } from "../types";
+import { isProduction } from "../helpers/is-production";
 
 export const setupAppFactory = (
   host: string | undefined,
@@ -20,7 +21,7 @@ export const setupAppFactory = (
 
     // If not on Glitch or Production, create a smee URL
     if (
-      process.env.NODE_ENV !== "production" &&
+      !isProduction() &&
       !(
         process.env.PROJECT_DOMAIN ||
         process.env.WEBHOOK_PROXY_URL ||

src/bin/read-cli-options.ts

@@ -44,7 +44,7 @@ export function readCliOptions(
     .option(
       "--log-format <format>",
       'One of: "pretty", "json"',
-      process.env.LOG_FORMAT || "pretty"
+      process.env.LOG_FORMAT
     )
     .option(
       "--log-level-in-string",

src/bin/read-env-options.ts

@@ -6,6 +6,9 @@ export function readEnvOptions(
 ) {
   const privateKey = getPrivateKey({ env });
 
+  const logFormat =
+    env.LOG_FORMAT || (env.NODE_ENV === "production" ? "json" : "pretty");
+
   return {
     args: [],
     privateKey: (privateKey && privateKey.toString()) || undefined,
@@ -16,7 +19,7 @@ export function readEnvOptions(
     webhookPath: env.WEBHOOK_PATH,
     webhookProxy: env.WEBHOOK_PROXY_URL,
     logLevel: env.LOG_LEVEL as LogLevel,
-    logFormat: env.LOG_FORMAT as PinoOptions["logFormat"],
+    logFormat: logFormat as PinoOptions["logFormat"],
     logLevelInString: env.LOG_LEVEL_IN_STRING === "true",
     logMessageKey: env.LOG_MESSAGE_KEY,
     sentryDsn: env.SENTRY_DSN,

src/helpers/get-log.ts

@@ -14,7 +14,7 @@
  * app.log.fatal("Goodbye, cruel world!");
  * ```
  */
-import pino, { LoggerOptions } from "pino";
+import pino, { Logger, LoggerOptions } from "pino";
 import { getTransformStream, Options, LogLevel } from "@probot/pino";
 
 export type GetLogOptions = {
@@ -22,7 +22,7 @@ export type GetLogOptions = {
   logMessageKey?: string;
 } & Options;
 
-export function getLog(options: GetLogOptions = {}) {
+export function getLog(options: GetLogOptions = {}): Logger {
   const { level, logMessageKey, ...getTransformStreamOptions } = options;
 
   const pinoOptions: LoggerOptions = {

src/helpers/is-production.ts

@@ -0,0 +1,3 @@
+export function isProduction() {
+  return process.env.NODE_ENV === "production";
+}

src/run.ts

@@ -9,6 +9,7 @@ import { readEnvOptions } from "./bin/read-env-options";
 import { Server } from "./server/server";
 import { defaultApp } from "./apps/default";
 import { resolveAppFunction } from "./helpers/resolve-app-function";
+import { isProduction } from "./helpers/is-production";
 
 type AdditionalOptions = {
   env: Record<string, string | undefined>;
@@ -85,7 +86,7 @@ export async function run(
   let server: Server;
 
   if (!appId || !privateKey) {
-    if (process.env.NODE_ENV === "production") {
+    if (isProduction()) {
       if (!appId) {
         throw new Error(
           "App ID is missing, and is required to run in production mode. " +

test/create-probot.test.ts

@@ -1,5 +1,5 @@
-import SonicBoom from "sonic-boom";
 import { createProbot, Probot } from "../src";
+import { captureLogOutput } from "./helpers/capture-log-output";
 
 const env = {
   APP_ID: "1",
@@ -64,30 +64,20 @@ describe("createProbot", () => {
     expect(probot.log.level).toEqual("trace");
   });
 
-  test("env, logger message key", () => {
-    let outputData = "";
+  test("env, logger message key", async () => {
+    const outputData = await captureLogOutput(() => {
+      const probot = createProbot({
+        env: {
+          ...env,
+          LOG_LEVEL: "info",
+          LOG_FORMAT: "json",
+          LOG_MESSAGE_KEY: "myMessage",
+        },
+        defaults: { logLevel: "trace" },
+      });
 
-    const sbWrite = SonicBoom.prototype.write;
-    SonicBoom.prototype.write = function (data) {
-      outputData += data;
-    };
-
-    const probot = createProbot({
-      env: {
-        ...env,
-        LOG_LEVEL: "info",
-        LOG_FORMAT: "json",
-        LOG_MESSAGE_KEY: "myMessage",
-      },
-      defaults: { logLevel: "trace" },
+      probot.log.info("Ciao");
     });
-
-    probot.log.info("Ciao");
-
-    try {
-      expect(JSON.parse(outputData).myMessage).toEqual("Ciao");
-    } finally {
-      SonicBoom.prototype.write = sbWrite;
-    }
+    expect(JSON.parse(outputData).myMessage).toEqual("Ciao");
   });
 });

test/helpers/capture-log-output.ts

@@ -0,0 +1,18 @@
+import SonicBoom from "sonic-boom";
+
+export async function captureLogOutput(action: () => any): Promise<string> {
+  let outputData = "";
+
+  const sbWrite = SonicBoom.prototype.write;
+  SonicBoom.prototype.write = function (data) {
+    outputData += data;
+  };
+
+  try {
+    await action();
+
+    return outputData;
+  } finally {
+    SonicBoom.prototype.write = sbWrite;
+  }
+}

test/helpers/is-production.test.ts

@@ -0,0 +1,16 @@
+import { isProduction } from "../../src/helpers/is-production";
+
+describe("isProduction", () => {
+  it("returns true if the NODE_ENV is set to production", () => {
+    process.env.NODE_ENV = "production";
+    expect(isProduction()).toBe(true);
+  });
+
+  it.each([undefined, "dev", "test", ""])(
+    "returns false if the NODE_ENV is set to %s",
+    (value) => {
+      process.env.NODE_ENV = value;
+      expect(isProduction()).toBe(false);
+    }
+  );
+});

test/run.test.ts

@@ -1,4 +1,3 @@
-import Stream from "stream";
 import path = require("path");
 
 import request from "supertest";
@@ -6,21 +5,14 @@ import { sign } from "@octokit/webhooks-methods";
 
 import { Probot, run, Server } from "../src";
 
+import { captureLogOutput } from "./helpers/capture-log-output";
+
 // tslint:disable:no-empty
 describe("run", () => {
   let server: Server;
-  let output: any;
   let env: NodeJS.ProcessEnv;
 
-  const streamLogsToOutput = new Stream.Writable({ objectMode: true });
-  streamLogsToOutput._write = (object, encoding, done) => {
-    output.push(JSON.parse(object));
-    done();
-  };
-
   beforeEach(() => {
-    // Clear log output
-    output = [];
     env = {
       APP_ID: "1",
       PRIVATE_KEY_PATH: path.join(
@@ -77,6 +69,22 @@ describe("run", () => {
         resolve(null);
       });
     });
+
+    it("defaults to JSON logs if NODE_ENV is set to 'production'", async () => {
+      const outputData = await captureLogOutput(async () => {
+        env.NODE_ENV = "production";
+
+        server = await run(
+          (app) => {
+            app.log.fatal("test");
+          },
+          { env }
+        );
+        await server.stop();
+      });
+
+      expect(outputData).toMatch(/"msg":"test"/);
+    });
   });
 
   describe("webhooks", () => {