fix(gcf-utils): Revert binding trigger information to logger (#809)

* Revert "feat(gcf-utils): add trigger info as bindings to all log statements (#796)" This reverts commit 157c768. * keep JSDocs and message changes
googleapis · Aug 6, 2020 · e9c42b3 · e9c42b3
1 parent 92401b2
commit e9c42b3
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 193 deletions.
diff --git a/packages/gcf-utils/src/gcf-utils.ts b/packages/gcf-utils/src/gcf-utils.ts
@@ -54,7 +54,7 @@ export interface WrapOptions {
   logging: boolean;
 }
 
-export let logger: GCFLogger = initLogger();
+export const logger: GCFLogger = initLogger();
 
 export interface CronPayload {
   repository: {
@@ -181,19 +181,6 @@ export class GCFBootstrapper {
     return TriggerType.UNKNOWN;
   }
 
-  /**
-   * Binds the given properties to the logger so that every logged statement
-   * includes these properties.
-   *
-   * NOTE: statements logged before this method is called will not include
-   * this information
-   *
-   * @param bindings object containing properties to bind to the logger
-   */
-  private static bindPropertiesToLogger(bindings: {}) {
-    logger = logger.child(bindings);
-  }
-
   gcf(
     appFn: ApplicationFunction,
     wrapOptions?: WrapOptions
@@ -213,9 +200,7 @@ export class GCFBootstrapper {
         taskId
       );
 
-      const triggerInfo = buildTriggerInfo(triggerType, id, request.body);
-      GCFBootstrapper.bindPropertiesToLogger(triggerInfo);
-      logger.metric(`Execution started by ${triggerType}`);
+      logger.metric(buildTriggerInfo(triggerType, id, request.body));
 
       try {
         if (triggerType === TriggerType.UNKNOWN) {

diff --git a/packages/gcf-utils/src/logging/gcf-logger.ts b/packages/gcf-utils/src/logging/gcf-logger.ts
@@ -22,11 +22,6 @@ interface LogFn {
   (obj: object, msg?: string, ...args: any[]): void;
 }
 
-interface Bindings {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  [key: string]: any;
-}
-
 /**
  * A logger standardized logger for Google Cloud Functions
  */
@@ -61,47 +56,16 @@ export interface GCFLogger {
    */
   metric: LogFn;
 
-  /**
-   * Create a child logger
-   * @param bindings static properties that will be included
-   * in all statements logged from the child logger
-   */
-  child: {(bindings: Bindings): GCFLogger};
-
-  /**
-   * Get the bindings associated with this logger
-   */
-  bindings: {(): Bindings};
-
   /**
    * Synchronously flush the buffer for this logger.
    * NOTE: Only supported for SonicBoom destinations
    */
   flushSync: {(): void};
 }
 
-/**
- * GCFLogger customization options
- */
-export interface GCFLoggerOptions {
-  /**
-   * An alternate destination to write logs.
-   * Default is stdout wrapped in SonicBoom
-   */
-  destination?: NodeJS.WritableStream | SonicBoom;
-
-  /**
-   * static properties that will be included
-   * in all statements logged
-   */
-  bindings?: Bindings;
-}
-
-/**
- * Initialize a GCFLogger instance
- * @param options options for instance initialization
- */
-export function initLogger(options?: GCFLoggerOptions): GCFLogger {
+export function initLogger(
+  dest?: NodeJS.WritableStream | SonicBoom
+): GCFLogger {
   const DEFAULT_LOG_LEVEL = 'trace';
   const defaultOptions: pino.LoggerOptions = {
     formatters: {
@@ -116,58 +80,26 @@ export function initLogger(options?: GCFLoggerOptions): GCFLogger {
     level: DEFAULT_LOG_LEVEL,
   };
 
-  const destination = options?.destination || pino.destination({sync: true});
-  let logger = pino(defaultOptions, destination);
-
-  if (options?.bindings) {
-    logger = logger.child(options.bindings);
-  }
-
-  bindPinoFunctions(logger);
+  dest = dest || pino.destination({sync: true});
+  const logger = pino(defaultOptions, dest);
+  Object.keys(logger).map(prop => {
+    if (logger[prop] instanceof Function) {
+      logger[prop] = logger[prop].bind(logger);
+    }
+  });
 
   const flushSync = () => {
     // flushSync is only available for SonicBoom,
     // which is the default destination wrapper for GCFLogger
-    if (destination instanceof SonicBoom) {
-      destination.flushSync();
+    if (dest instanceof SonicBoom) {
+      dest.flushSync();
     }
   };
 
-  return pinoToGCFLogger(logger, flushSync);
-}
-
-/**
- * Binds the given logger instance to Pino logger functions
- * @param logger instance to bind
- */
-function bindPinoFunctions(logger: pino.Logger): pino.Logger {
-  Object.keys(logger).map(prop => {
-    if (logger[prop] instanceof Function) {
-      logger[prop] = logger[prop].bind(logger);
-    }
-  });
-  return logger;
-}
-
-/**
- * Converts a Pino logger instance to a GCFLogger instance
- * @param pinoLogger pino logger to convert
- * @param flushSync the flushSync function to apply to the returned GCFLogger
- */
-function pinoToGCFLogger(
-  pinoLogger: pino.Logger,
-  flushSync: {(): void}
-): GCFLogger {
   return {
-    ...pinoLogger,
-    metric: pinoLogger.metric.bind(pinoLogger),
+    ...logger,
+    metric: logger.metric.bind(logger),
     flushSync: flushSync,
-    bindings: pinoLogger.bindings,
-    child: (bindings: Bindings) => {
-      const child = pinoLogger.child(bindings);
-      bindPinoFunctions(child);
-      return pinoToGCFLogger(child, flushSync);
-    },
   };
 }
 

diff --git a/packages/gcf-utils/src/logging/trigger-info-builder.ts b/packages/gcf-utils/src/logging/trigger-info-builder.ts
@@ -18,7 +18,8 @@ import crypto from 'crypto';
 /**
  * Information on GCF execution trigger
  */
-export interface TriggerInfo {
+interface TriggerInfo {
+  message: string;
   trigger: {
     trigger_type: TriggerType;
     trigger_sender?: string;
@@ -55,6 +56,7 @@ export function buildTriggerInfo(
   const UNKNOWN = 'UNKNOWN';
 
   const triggerInfo: TriggerInfo = {
+    message: `Execution started by ${triggerType}`,
     trigger: {
       trigger_type: triggerType,
     },

diff --git a/packages/gcf-utils/test/gcf-bootstrapper.ts b/packages/gcf-utils/test/gcf-bootstrapper.ts
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {GCFBootstrapper, WrapOptions, logger} from '../src/gcf-utils';
+import {GCFBootstrapper, WrapOptions} from '../src/gcf-utils';
 import {describe, beforeEach, afterEach, it} from 'mocha';
 import {GitHubAPI} from 'probot/lib/github';
 import {Options} from 'probot';
@@ -367,26 +367,4 @@ describe('GCFBootstrapper', () => {
       assert.strictEqual(latest, 'projects/foo/secrets/bar');
     });
   });
-
-  describe('bindPropertiesToLogger', () => {
-    it('binds given properties', () => {
-      const triggerInfoWithoutMessage = {
-        trigger: {
-          trigger_type: 'GITHUB_WEBHOOK',
-          trigger_sender: 'some sender',
-          payload_hash: '123456',
-
-          trigger_source_repo: {
-            owner: 'foo owner',
-            owner_type: 'Org',
-            repo_name: 'bar name',
-            url: 'some url',
-          },
-        },
-      };
-
-      GCFBootstrapper['bindPropertiesToLogger'](triggerInfoWithoutMessage);
-      assert.deepEqual(logger.bindings(), triggerInfoWithoutMessage);
-    });
-  });
 });
diff --git a/packages/gcf-utils/test/gcf-logger.ts b/packages/gcf-utils/test/gcf-logger.ts
@@ -26,9 +26,7 @@ describe('GCFLogger', () => {
   });
   describe('logger instance', () => {
     let destination: ObjectWritableMock;
-    let loggerNoBindings: GCFLogger & {[key: string]: Function};
-    let loggerWithBindings: GCFLogger & {[key: string]: Function};
-    const bindings = {foo: 'bar-binding'};
+    let logger: GCFLogger & {[key: string]: Function};
 
     function readLogsAsObjects(writeStream: ObjectWritableMock): LogLine[] {
       try {
@@ -43,13 +41,13 @@ describe('GCFLogger', () => {
     function testAllLevels() {
       for (const level of Object.keys(logLevels)) {
         it(`logs ${level} level string`, () => {
-          loggerNoBindings[level]('hello world');
+          logger[level]('hello world');
           const loggedLines: LogLine[] = readLogsAsObjects(destination);
           validateLogs(loggedLines, 1, ['hello world'], [], logLevels[level]);
         });
 
         it(`logs ${level} level json`, () => {
-          loggerNoBindings[level]({hello: 'world'});
+          logger[level]({hello: 'world'});
           const loggedLines: LogLine[] = readLogsAsObjects(destination);
           validateLogs(
             loggedLines,
@@ -59,41 +57,12 @@ describe('GCFLogger', () => {
             logLevels[level]
           );
         });
-
-        it(`logs ${level} level string with bindings`, () => {
-          loggerWithBindings[level]('hello world');
-          const loggedLines: LogLine[] = readLogsAsObjects(destination);
-          validateLogs(
-            loggedLines,
-            1,
-            ['hello world'],
-            [bindings],
-            logLevels[level]
-          );
-        });
-
-        it(`logs ${level} level json with bindings`, () => {
-          loggerWithBindings[level]({hello: 'world'});
-          const loggedLines: LogLine[] = readLogsAsObjects(destination);
-          validateLogs(
-            loggedLines,
-            1,
-            [],
-            [{...bindings, hello: 'world'}],
-            logLevels[level]
-          );
-        });
       }
     }
 
     beforeEach(() => {
       destination = new ObjectWritableMock();
-      loggerNoBindings = initLogger({destination}) as GCFLogger & {
-        [key: string]: Function;
-      };
-      loggerWithBindings = loggerNoBindings.child(bindings) as GCFLogger & {
-        [key: string]: Function;
-      };
+      logger = initLogger(destination) as GCFLogger & {[key: string]: Function};
     });
 
     testAllLevels();

diff --git a/packages/gcf-utils/test/integration/gcf-logger-integration.ts b/packages/gcf-utils/test/integration/gcf-logger-integration.ts
@@ -20,9 +20,7 @@ import SonicBoom from 'sonic-boom';
 import fs from 'fs';
 
 describe('GCFLogger Integration', () => {
-  let loggerNoBindings: GCFLogger & {[key: string]: Function};
-  let loggerWithBindings: GCFLogger & {[key: string]: Function};
-  const bindings = {foo: 'bar-binding'};
+  let logger: GCFLogger & {[key: string]: Function};
   const testStreamPath = './test-stream.txt';
   let destination: SonicBoom;
 
@@ -44,7 +42,7 @@ describe('GCFLogger Integration', () => {
   function testAllLevels() {
     for (const level of Object.keys(logLevels)) {
       it(`logs ${level} level string`, done => {
-        loggerNoBindings[level]('hello world');
+        logger[level]('hello world');
         destination.on('ready', () => {
           const loggedLines: LogLine[] = readLogsAsObjects(destination);
           validateLogs(loggedLines, 1, ['hello world'], [], logLevels[level]);
@@ -53,7 +51,7 @@ describe('GCFLogger Integration', () => {
       });
 
       it(`logs ${level} level json`, done => {
-        loggerNoBindings[level]({hello: 'world'});
+        logger[level]({hello: 'world'});
         destination.on('ready', () => {
           const loggedLines: LogLine[] = readLogsAsObjects(destination);
           validateLogs(
@@ -66,41 +64,12 @@ describe('GCFLogger Integration', () => {
           done();
         });
       });
-
-      it(`logs ${level} level string with bindings`, () => {
-        loggerWithBindings[level]('hello world');
-        const loggedLines: LogLine[] = readLogsAsObjects(destination);
-        validateLogs(
-          loggedLines,
-          1,
-          ['hello world'],
-          [bindings],
-          logLevels[level]
-        );
-      });
-
-      it(`logs ${level} level json with bindings`, () => {
-        loggerWithBindings[level]({hello: 'world'});
-        const loggedLines: LogLine[] = readLogsAsObjects(destination);
-        validateLogs(
-          loggedLines,
-          1,
-          [],
-          [{...bindings, hello: 'world'}],
-          logLevels[level]
-        );
-      });
     }
   }
 
   beforeEach(() => {
     destination = pino.destination(testStreamPath);
-    loggerNoBindings = initLogger({destination: destination}) as GCFLogger & {
-      [key: string]: Function;
-    };
-    loggerWithBindings = loggerNoBindings.child(bindings) as GCFLogger & {
-      [key: string]: Function;
-    };
+    logger = initLogger(destination) as GCFLogger & {[key: string]: Function};
   });
 
   testAllLevels();