Merge pull request #542 from forcedotcom/ew/rotating-log-file-v3

RodEsp · web-flow · commit eb7d6fe76795 · 2022-03-24T12:30:09.000-04:00
Rotating file logs V3
diff --git a/messages/envVars.md b/messages/envVars.md
@@ -94,6 +94,14 @@ Set to true to send messages resulting from failed Salesforce CLI commands to st
 
 Level of messages that the CLI writes to the log file. Valid values are trace, debug, info, warn, error, fatal. Default value is warn.
 
+# sfdxLogRotationCount
+
+The default rotation period for logs. Example '1d' will rotate logs daily (at midnight).
+
+# sfdxLogRotationPeriod
+
+The number of backup rotated log files to keep. Example: '3' will have the base sf.log file, and the past 3 (period) log files.
+
 # sfdxMaxQueryLimit
 
 Maximum number of Salesforce records returned by a CLI command. Default value is 10,000. Overrides the maxQueryLimit configuration value.
@@ -222,6 +230,14 @@ Set to true to send messages resulting from failed Salesforce CLI commands to st
 
 Level of messages that the CLI writes to the log file. Valid values are trace, debug, info, warn, error, fatal. Default value is warn.
 
+# sfLogRotationCount
+
+The default rotation period for logs. Example '1d' will rotate logs daily (at midnight).
+
+# sfLogRotationPeriod
+
+The number of backup rotated log files to keep. Example: '3' will have the base sf.log file, and the past 3 (period) log files.
+
 # sfMaxQueryLimit
 
 Maximum number of Salesforce records returned by a CLI command. Default value is 10,000. Overrides the maxQueryLimit configuration variable.
diff --git a/src/config/envVars.ts b/src/config/envVars.ts
@@ -38,6 +38,8 @@ export enum EnvironmentVariable {
   'SFDX_INSTANCE_URL' = 'SFDX_INSTANCE_URL',
   'SFDX_JSON_TO_STDOUT' = 'SFDX_JSON_TO_STDOUT',
   'SFDX_LOG_LEVEL' = 'SFDX_LOG_LEVEL',
+  'SFDX_LOG_ROTATION_COUNT' = 'SFDX_LOG_ROTATION_COUNT',
+  'SFDX_LOG_ROTATION_PERIOD' = 'SFDX_LOG_ROTATION_PERIOD',
   'SFDX_MAX_QUERY_LIMIT' = 'SFDX_MAX_QUERY_LIMIT',
   'SFDX_MDAPI_TEMP_DIR' = 'SFDX_MDAPI_TEMP_DIR',
   'SFDX_NPM_REGISTRY' = 'SFDX_NPM_REGISTRY',
@@ -70,6 +72,8 @@ export enum EnvironmentVariable {
   'SF_INSTANCE_URL' = 'SF_INSTANCE_URL',
   'SF_JSON_TO_STDOUT' = 'SF_JSON_TO_STDOUT',
   'SF_LOG_LEVEL' = 'SF_LOG_LEVEL',
+  'SF_LOG_ROTATION_COUNT' = 'SF_LOG_ROTATION_COUNT',
+  'SF_LOG_ROTATION_PERIOD' = 'SF_LOG_ROTATION_PERIOD',
   'SF_MAX_QUERY_LIMIT' = 'SF_MAX_QUERY_LIMIT',
   'SF_MDAPI_TEMP_DIR' = 'SF_MDAPI_TEMP_DIR',
   'SF_NPM_REGISTRY' = 'SF_NPM_REGISTRY',
@@ -195,6 +199,14 @@ export const SUPPORTED_ENV_VARS: EnvType = {
     description: getMessage(EnvironmentVariable.SFDX_LOG_LEVEL),
     synonymOf: EnvironmentVariable.SF_LOG_LEVEL,
   },
+  [EnvironmentVariable.SFDX_LOG_ROTATION_COUNT]: {
+    description: getMessage(EnvironmentVariable.SFDX_LOG_ROTATION_COUNT),
+    synonymOf: EnvironmentVariable.SF_LOG_ROTATION_COUNT,
+  },
+  [EnvironmentVariable.SFDX_LOG_ROTATION_PERIOD]: {
+    description: getMessage(EnvironmentVariable.SFDX_LOG_ROTATION_PERIOD),
+    synonymOf: EnvironmentVariable.SF_LOG_ROTATION_PERIOD,
+  },
   [EnvironmentVariable.SFDX_MAX_QUERY_LIMIT]: {
     description: getMessage(EnvironmentVariable.SFDX_MAX_QUERY_LIMIT),
     synonymOf: EnvironmentVariable.SF_MAX_QUERY_LIMIT,
@@ -326,6 +338,14 @@ export const SUPPORTED_ENV_VARS: EnvType = {
     description: getMessage(EnvironmentVariable.SF_LOG_LEVEL),
     synonymOf: null,
   },
+  [EnvironmentVariable.SF_LOG_ROTATION_COUNT]: {
+    description: getMessage(EnvironmentVariable.SF_LOG_ROTATION_COUNT),
+    synonymOf: null,
+  },
+  [EnvironmentVariable.SF_LOG_ROTATION_PERIOD]: {
+    description: getMessage(EnvironmentVariable.SF_LOG_ROTATION_PERIOD),
+    synonymOf: null,
+  },
   [EnvironmentVariable.SF_MAX_QUERY_LIMIT]: {
     description: getMessage(EnvironmentVariable.SF_MAX_QUERY_LIMIT),
     synonymOf: null,
diff --git a/src/logger.ts b/src/logger.ts
@@ -12,7 +12,7 @@ import * as fs from 'fs';
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
 import * as Bunyan from '@salesforce/bunyan';
-import { parseJson, parseJsonMap } from '@salesforce/kit';
+import { Env, parseJson, parseJsonMap } from '@salesforce/kit';
 import {
   Dictionary,
   ensure,
@@ -221,6 +221,21 @@ export class Logger {
   // The sfdx root logger singleton
   private static rootLogger?: Logger;
 
+  /**
+   * The default rotation period for logs. Example '1d' will rotate logs daily (at midnight).
+   * See 'period' docs here: https://github.com/forcedotcom/node-bunyan#stream-type-rotating-file
+   */
+
+  public readonly logRotationPeriod = new Env().getString('SF_LOG_ROTATION_PERIOD') || '1d';
+
+  /**
+   * The number of backup rotated log files to keep.
+   * Example: '3' will have the base sf.log file, and the past 3 (period) log files.
+   * See 'count' docs here: https://github.com/forcedotcom/node-bunyan#stream-type-rotating-file
+   */
+
+  public readonly logRotationCount = new Env().getNumber('SF_LOG_ROTATION_COUNT') || 2;
+
   /**
    * Whether debug is enabled for this Logger.
    */
@@ -418,14 +433,14 @@ export class Logger {
       !this.bunyan.streams.find(
         // No bunyan typings
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        (stream: any) => stream.type === 'file' && stream.path === logFile
+        (stream: any) => stream.type === 'rotating-file' && stream.path === logFile
       )
     ) {
-      // TODO: rotating-file
-      // https://github.com/trentm/node-bunyan#stream-type-rotating-file
       this.addStream({
-        type: 'file',
+        type: 'rotating-file',
         path: logFile,
+        period: this.logRotationPeriod,
+        count: this.logRotationCount,
         level: this.bunyan.level() as number,
       });
     }
@@ -460,14 +475,14 @@ export class Logger {
       !this.bunyan.streams.find(
         // No bunyan typings
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        (stream: any) => stream.type === 'file' && stream.path === logFile
+        (stream: any) => stream.type === 'rotating-file' && stream.path === logFile
       )
     ) {
-      // TODO: rotating-file
-      // https://github.com/trentm/node-bunyan#stream-type-rotating-file
       this.addStream({
-        type: 'file',
+        type: 'rotating-file',
         path: logFile,
+        period: this.logRotationPeriod,
+        count: this.logRotationCount,
         level: this.bunyan.level() as number,
       });
     }
diff --git a/test/unit/loggerTest.ts b/test/unit/loggerTest.ts
@@ -20,6 +20,8 @@ const $$ = testSetup();
 
 describe('Logger', () => {
   const sfdxEnv = process.env.SFDX_ENV;
+  const logRotationPeriodBackup = process.env.SF_LOG_ROTATION_PERIOD;
+  const logRotationCountBackup = process.env.SF_LOG_ROTATION_COUNT;
 
   beforeEach(async () => {
     process.env.SFDX_ENV = 'test';
@@ -32,7 +34,9 @@ describe('Logger', () => {
 
   afterEach(() => {
     Logger.destroyRoot();
-    process.env.SFDX_ENV = sfdxEnv;
+    if (sfdxEnv) process.env.SFDX_ENV = sfdxEnv;
+    if (logRotationPeriodBackup) process.env.SF_LOG_ROTATION_PERIOD = logRotationPeriodBackup;
+    if (logRotationCountBackup) process.env.SF_LOG_ROTATION_COUNT = logRotationCountBackup;
   });
 
   describe('constructor', () => {
@@ -110,11 +114,25 @@ describe('Logger', () => {
       expect(utilAccessStub.firstCall.args[0]).to.equal(testLogFile);
       expect(utilWriteFileStub.called).to.be.false;
       const addStreamArgs = addStreamStub.firstCall.args[0];
-      expect(addStreamArgs).to.have.property('type', 'file');
+      expect(addStreamArgs).to.have.property('type', 'rotating-file');
       expect(addStreamArgs).to.have.property('path', testLogFile);
       expect(addStreamArgs).to.have.property('level', logger.getLevel());
     });
 
+    it('should allow log rotation count and period overrides', async () => {
+      process.env.SF_LOG_ROTATION_PERIOD = '1w';
+      process.env.SF_LOG_ROTATION_COUNT = '3';
+
+      utilAccessStub.returns(Promise.resolve({}));
+      const logger = new Logger('testing-env-vars');
+      const addStreamStub = $$.SANDBOX.stub(logger, 'addStream');
+      await logger.addLogFileStream(testLogFile);
+
+      const addStreamArgs = addStreamStub.firstCall.args[0];
+      expect(addStreamArgs).to.have.property('period', '1w');
+      expect(addStreamArgs).to.have.property('count', 3);
+    });
+
     it('should create a new log file and all directories if nonexistent', async () => {
       utilAccessStub.throws();
       const logger = new Logger('testLogger');
