fix: Use defaultCallback in LoggingCommon class (#672)

* fix: Use defaultCallback from LoggingCommon class * Remove obsolete test * Fax test failure * Add test to validate callback is called
googleapis · Mar 9, 2022 · 4bc7baa · 4bc7baa
1 parent 52676bf
commit 4bc7baa
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 19 deletions.
diff --git a/src/common.ts b/src/common.ts
@@ -115,6 +115,7 @@ export class LoggingCommon {
   private serviceContext: ServiceContext | undefined;
   private prefix: string | undefined;
   private labels: object | undefined;
+  private defaultCallback?: Callback;
   // LOGGING_TRACE_KEY is Cloud Logging specific and has the format:
   // logging.googleapis.com/trace
   static readonly LOGGING_TRACE_KEY = LOGGING_TRACE_KEY;
@@ -139,13 +140,12 @@ export class LoggingCommon {
       // 250,000 has been chosen to keep us comfortably within the
       // 256,000 limit.
       maxEntrySize: options.maxEntrySize || 250000,
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      defaultWriteDeleteCallback: options.defaultCallback,
     });
     this.resource = options.resource;
     this.serviceContext = options.serviceContext;
     this.prefix = options.prefix;
     this.labels = options.labels;
+    this.defaultCallback = options.defaultCallback;
   }
 
   log(
@@ -257,7 +257,10 @@ export class LoggingCommon {
     }
 
     const entry = this.stackdriverLog.entry(entryMetadata, data);
-    this.stackdriverLog[stackdriverLevel](entry, callback);
+    this.stackdriverLog[stackdriverLevel](
+      entry,
+      this.defaultCallback ?? callback
+    );
   }
 }
 

diff --git a/src/index.ts b/src/index.ts
@@ -28,7 +28,6 @@ import {
   ServiceContext,
   LoggingOptions,
 } from '@google-cloud/logging';
-import {ApiResponseCallback} from '@google-cloud/logging/build/src/log';
 
 const LEVEL = Symbol.for('level');
 
@@ -89,7 +88,7 @@ export interface Options extends LoggingOptions {
 
   // A default global callback to be used for {@link LoggingWinston#log} when callback is
   // not supplied by caller in function parameters
-  defaultCallback?: ApiResponseCallback;
+  defaultCallback?: Callback;
 }
 
 /**

diff --git a/test/common.ts b/test/common.ts
@@ -158,20 +158,6 @@ describe('logging-common', () => {
       assert.deepStrictEqual(fakeLogOptions_, {
         removeCircular: true,
         maxEntrySize: 250000,
-        defaultWriteDeleteCallback: undefined,
-      });
-    });
-
-    it('should set default callback', () => {
-      const optionsWithDefaultCallback = Object.assign({}, OPTIONS, {
-        defaultCallback: () => {},
-      });
-      new loggingCommonLib.LoggingCommon(optionsWithDefaultCallback);
-
-      assert.deepStrictEqual(fakeLogOptions_, {
-        removeCircular: true,
-        maxEntrySize: 250000,
-        defaultWriteDeleteCallback: optionsWithDefaultCallback.defaultCallback,
       });
     });
 

diff --git a/test/stackdriver-trace-integration.ts b/test/stackdriver-trace-integration.ts
@@ -32,6 +32,8 @@ describe('Stackdriver Trace Log Correlation', () => {
   let setCurrentContextId: (id: string) => void;
   // The Stackdriver Logging Winston transport library.
   let loggingWinstonLib: typeof loggingWinstonLibTypes;
+  // The flag indicating if callback was called or not
+  let isCallbackCalled: boolean;
 
   class FakeLogging {
     constructor() {}
@@ -63,6 +65,7 @@ describe('Stackdriver Trace Log Correlation', () => {
 
   beforeEach(() => {
     seenContextIds.length = 0;
+    isCallbackCalled = false;
     setCurrentContextId = (() => {
       let currentContextId: string;
       global._google_trace_agent = {
@@ -128,6 +131,22 @@ describe('Stackdriver Trace Log Correlation', () => {
     });
   });
 
+  it('Calls default callback when present', done => {
+    const transport = new loggingWinstonLib.LoggingWinston({
+      defaultCallback: () => {
+        isCallbackCalled = true;
+      },
+    });
+    const logger = winston.createLogger({
+      transports: [transport],
+    });
+    logger.log({level: 'info', message: 'hello'});
+    setImmediate(() => {
+      assert.strictEqual(isCallbackCalled, true);
+      done();
+    });
+  });
+
   [null, {}, {getWriterProjectId: () => 'project1'}].forEach(testCase => {
     it(`Doesn't crash when a non-compatible Trace Agent is present: ${testCase}`, done => {
       global._google_trace_agent = testCase;