From 7d2074a95aaf1198bbe1c7b2ced00729b93411b8 Mon Sep 17 00:00:00 2001
From: Josh Dover <me@joshdover.com>
Date: Tue, 28 Jan 2020 07:34:27 -0700
Subject: [PATCH] Add Appender.receiveAllLevels option to fix LegacyAppender
 (#55752)

---
 .../logging/appenders/legacy_appender.ts      |  6 ++
 .../server/logging/appenders/appenders.ts     |  6 ++
 src/core/server/logging/logger.test.ts        | 82 +++++++++++++++++++
 src/core/server/logging/logger.ts             |  8 +-
 4 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/src/core/server/legacy/logging/appenders/legacy_appender.ts b/src/core/server/legacy/logging/appenders/legacy_appender.ts
index 6d82d929e7daa..0c2f4ce93c3b8 100644
--- a/src/core/server/legacy/logging/appenders/legacy_appender.ts
+++ b/src/core/server/legacy/logging/appenders/legacy_appender.ts
@@ -33,6 +33,12 @@ export class LegacyAppender implements DisposableAppender {
     legacyLoggingConfig: schema.any(),
   });
 
+  /**
+   * Sets {@link Appender.receiveAllLevels} because legacy does its own filtering based on the legacy logging
+   * configuration.
+   */
+  public readonly receiveAllLevels = true;
+
   private readonly loggingServer: LegacyLoggingServer;
 
   constructor(legacyLoggingConfig: Readonly<LegacyVars>) {
diff --git a/src/core/server/logging/appenders/appenders.ts b/src/core/server/logging/appenders/appenders.ts
index 3aa86495e4d82..871acb8c465ca 100644
--- a/src/core/server/logging/appenders/appenders.ts
+++ b/src/core/server/logging/appenders/appenders.ts
@@ -42,6 +42,12 @@ export type AppenderConfigType = TypeOf<typeof appendersSchema>;
  */
 export interface Appender {
   append(record: LogRecord): void;
+
+  /**
+   * Used to signal to `Logger` that log level filtering should be ignored for this appender. Defaults to `false`.
+   * @deprecated Should be removed once the `LegacyAppender` is removed.
+   */
+  receiveAllLevels?: boolean;
 }
 
 /**
diff --git a/src/core/server/logging/logger.test.ts b/src/core/server/logging/logger.test.ts
index 026e24fc5df54..eeebb8ad5a0fa 100644
--- a/src/core/server/logging/logger.test.ts
+++ b/src/core/server/logging/logger.test.ts
@@ -410,3 +410,85 @@ test('passes log record to appenders only if log level is supported.', () => {
     });
   }
 });
+
+test('passes log record to appender with receiveAllLevels: true, regardless if log level is supported', () => {
+  const receiveAllAppender = { append: jest.fn(), receiveAllLevels: true };
+  const warnLogger = new BaseLogger(context, LogLevel.Warn, [receiveAllAppender], factory);
+
+  warnLogger.trace('trace-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(1);
+  expect(receiveAllAppender.append.mock.calls[0][0]).toMatchObject({
+    level: LogLevel.Trace,
+    message: 'trace-message',
+  });
+
+  warnLogger.debug('debug-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(2);
+  expect(receiveAllAppender.append.mock.calls[1][0]).toMatchObject({
+    level: LogLevel.Debug,
+    message: 'debug-message',
+  });
+
+  warnLogger.info('info-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(3);
+  expect(receiveAllAppender.append.mock.calls[2][0]).toMatchObject({
+    level: LogLevel.Info,
+    message: 'info-message',
+  });
+
+  warnLogger.warn('warn-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(4);
+  expect(receiveAllAppender.append.mock.calls[3][0]).toMatchObject({
+    level: LogLevel.Warn,
+    message: 'warn-message',
+  });
+
+  warnLogger.error('error-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(5);
+  expect(receiveAllAppender.append.mock.calls[4][0]).toMatchObject({
+    level: LogLevel.Error,
+    message: 'error-message',
+  });
+
+  warnLogger.fatal('fatal-message');
+  expect(receiveAllAppender.append).toHaveBeenCalledTimes(6);
+  expect(receiveAllAppender.append.mock.calls[5][0]).toMatchObject({
+    level: LogLevel.Fatal,
+    message: 'fatal-message',
+  });
+});
+
+test('passes log record to appender with receiveAllLevels: false, only if log level is supported', () => {
+  const notReceiveAllAppender = { append: jest.fn(), receiveAllLevels: false };
+  const warnLogger = new BaseLogger(context, LogLevel.Warn, [notReceiveAllAppender], factory);
+
+  warnLogger.trace('trace-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(0);
+
+  warnLogger.debug('debug-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(0);
+
+  warnLogger.info('info-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(0);
+
+  warnLogger.warn('warn-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(1);
+  expect(notReceiveAllAppender.append.mock.calls[0][0]).toMatchObject({
+    level: LogLevel.Warn,
+    message: 'warn-message',
+  });
+
+  warnLogger.error('error-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(2);
+  expect(notReceiveAllAppender.append.mock.calls[1][0]).toMatchObject({
+    level: LogLevel.Error,
+    message: 'error-message',
+  });
+
+  warnLogger.fatal('fatal-message');
+  expect(notReceiveAllAppender.append).toHaveBeenCalledTimes(3);
+  expect(notReceiveAllAppender.append.mock.calls[2][0]).toMatchObject({
+    level: LogLevel.Fatal,
+    message: 'fatal-message',
+  });
+});
diff --git a/src/core/server/logging/logger.ts b/src/core/server/logging/logger.ts
index ac79c1916c07b..ab6906ff5d684 100644
--- a/src/core/server/logging/logger.ts
+++ b/src/core/server/logging/logger.ts
@@ -136,12 +136,12 @@ export class BaseLogger implements Logger {
   }
 
   public log(record: LogRecord) {
-    if (!this.level.supports(record.level)) {
-      return;
-    }
+    const supportedLevel = this.level.supports(record.level);
 
     for (const appender of this.appenders) {
-      appender.append(record);
+      if (supportedLevel || appender.receiveAllLevels) {
+        appender.append(record);
+      }
     }
   }