fix: Setting maxEntrySize does not truncate big json payloads correct…

…ly (#1177)

* fix: Setting maxEntrySize does not truncate big json payloads correctly

* Change variable names

* Add more validations and adjust test

* Address PR comments

* Fix msg check to validate null and undefined

src/log.ts

@@ -59,6 +59,7 @@ export interface TailEntriesRequest {
 export interface LogOptions {
   removeCircular?: boolean;
   maxEntrySize?: number; // see: https://cloud.google.com/logging/quotas
+  jsonFieldsToTruncate?: string[];
 }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -86,6 +87,8 @@ export type DeleteCallback = ApiResponseCallback;
  * @param {boolean} [options.removeCircular] Replace circular references in
  *     logged objects with a string value, `[Circular]`. (Default: false)
  * @param {number} [options.maxEntrySize] A max entry size
+ * @param {string[]} [options.jsonFieldsToTruncate] A list of JSON properties at the given full path to be truncated.
+ *     Received values will be prepended to predefined list in the order received and duplicates discarded.
  *
  * @example
  * ```
@@ -100,6 +103,7 @@ class Log implements LogSeverityFunctions {
   maxEntrySize?: number;
   logging: Logging;
   name: string;
+  jsonFieldsToTruncate: string[];
 
   constructor(logging: Logging, name: string, options?: LogOptions) {
     options = options || {};
@@ -112,6 +116,35 @@ class Log implements LogSeverityFunctions {
      * @type {string}
      */
     this.name = this.formattedName_.split('/').pop()!;
+    this.jsonFieldsToTruncate = [
+      // Winston:
+      'jsonPayload.fields.metadata.structValue.fields.stack.stringValue',
+      // Bunyan:
+      'jsonPayload.fields.msg.stringValue',
+      'jsonPayload.fields.err.structValue.fields.stack.stringValue',
+      'jsonPayload.fields.err.structValue.fields.message.stringValue',
+      // All:
+      'jsonPayload.fields.message.stringValue',
+    ];
+
+    // Prepend all custom fields to be truncated to a list with defaults, thus
+    // custom fields will be truncated first. Make sure to filter out fields
+    // which are not in EntryJson.jsonPayload
+    if (
+      options.jsonFieldsToTruncate !== null &&
+      options.jsonFieldsToTruncate !== undefined
+    ) {
+      const filteredList = options.jsonFieldsToTruncate.filter(
+        str =>
+          str !== null &&
+          !this.jsonFieldsToTruncate.includes(str) &&
+          str.startsWith('jsonPayload')
+      );
+      const uniqueSet = new Set(filteredList);
+      this.jsonFieldsToTruncate = Array.from(uniqueSet).concat(
+        this.jsonFieldsToTruncate
+      );
+    }
   }
 
   /**
@@ -988,25 +1021,18 @@ class Log implements LogSeverityFunctions {
           Math.max(entry.textPayload.length - delta, 0)
         );
       } else {
-        const fieldsToTruncate = [
-          // Winston:
-          'jsonPayload.fields.metadata.structValue.fields.stack.stringValue',
-          // Bunyan:
-          'jsonPayload.fields.msg.stringValue',
-          'jsonPayload.fields.err.structValue.fields.stack.stringValue',
-          'jsonPayload.fields.err.structValue.fields.message.stringValue',
-          // All:
-          'jsonPayload.fields.message.stringValue',
-        ];
-        for (const field of fieldsToTruncate) {
+        for (const field of this.jsonFieldsToTruncate) {
           const msg: string = dotProp.get(entry, field, '');
-          if (msg !== '') {
+          if (msg !== null && msg !== undefined && msg !== '') {
             dotProp.set(
               entry,
               field,
               msg.slice(0, Math.max(msg.length - delta, 0))
             );
             delta -= Math.min(msg.length, delta);
+            if (delta <= 0) {
+              break;
+            }
           }
         }
       }

test/entry.ts

@@ -362,7 +362,7 @@ describe('Entry', () => {
         headers: {
           ['x-cloud-trace-context']: '1/1',
         },
-      } as any as http.IncomingMessage;
+      } as unknown as http.IncomingMessage;
       const json = entry.toStructuredJSON();
       assert.strictEqual(json[entryTypes.TRACE_KEY], 'projects//traces/1');
       assert.strictEqual(json[entryTypes.SPAN_ID_KEY], '1');

test/index.ts

@@ -1772,6 +1772,7 @@ describe('Logging', () => {
       logging = new Logging();
       sinon.stub(metadata, 'getDefaultResource').resolves({type: 'bar'});
       await logging.setDetectedResource();
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
       assert.strictEqual((logging.detectedResource as any).type, 'bar');
       sinon.restore();
     });

test/log.ts

@@ -31,6 +31,9 @@ describe('Log', () => {
   const PROJECT_ID = 'project-id';
   const FAKE_RESOURCE = 'fake-resource';
   const LOG_NAME = 'escaping/required/for/this/log-name';
+  const TRUNCATE_FIELD =
+    'jsonPayload.fields.metadata.structValue.fields.custom.stringValue';
+  const INVALID_TRUNCATE_FIELD = 'insertId';
   const LOG_NAME_ENCODED = encodeURIComponent(LOG_NAME);
   const LOG_NAME_FORMATTED = [
     'projects',
@@ -95,7 +98,17 @@ describe('Log', () => {
       },
     } as {} as Logging;
 
-    const options: LogOptions = {};
+    // Add some custom defined field to truncate which can be tested later - the idea is to
+    // see that constructor works properly and provides correct order of fields to be truncated.
+    // Also append same value twice to make sure that duplicates should be discarded.
+    // Adding illegal field to be truncated should be discared as well
+    const options: LogOptions = {
+      jsonFieldsToTruncate: [
+        INVALID_TRUNCATE_FIELD,
+        TRUNCATE_FIELD,
+        TRUNCATE_FIELD,
+      ],
+    };
     if (maxEntrySize) {
       options.maxEntrySize = maxEntrySize;
     }
@@ -633,6 +646,41 @@ describe('Log', () => {
       assert.ok(message.startsWith('hello world'));
       assert.ok(message.length < maxSize + entryMetaMaxLength);
     });
+
+    it('should not contin duplicate or illegal fields to be truncated and defaults should present', async () => {
+      assert.ok(log.jsonFieldsToTruncate.length > 1);
+      assert.ok(log.jsonFieldsToTruncate[0] === TRUNCATE_FIELD);
+      const notExists = log.jsonFieldsToTruncate.filter(
+        (str: string) => str === INVALID_TRUNCATE_FIELD
+      );
+      assert.strictEqual(notExists.length, 0);
+      const existOnce = log.jsonFieldsToTruncate.filter(
+        (str: string) => str === TRUNCATE_FIELD
+      );
+      assert.strictEqual(existOnce.length, 1);
+    });
+
+    it('should truncate custom defined field', async () => {
+      const maxSize = 300;
+      const entries = entriesFactory({
+        message: 'hello world'.padEnd(2000, '.'),
+        metadata: {
+          custom: 'custom world'.padEnd(2000, '.'),
+        },
+      });
+
+      log.maxEntrySize = maxSize;
+      log.truncateEntries(entries);
+
+      const message: string =
+        entries[0].jsonPayload!.fields!.message.stringValue!;
+      const custom: string =
+        entries[0].jsonPayload!.fields!.metadata.structValue!.fields!.custom
+          .stringValue!;
+      assert.ok(message.startsWith('hello world'));
+      assert.strictEqual(custom, '');
+      assert.ok(message.length < maxSize + entryMetaMaxLength);
+    });
   });
 
   describe('severity shortcuts', () => {