Skip to content

Commit

Permalink
chore: breaking changes for Logger
Browse files Browse the repository at this point in the history
  • Loading branch information
mshanemc committed Jul 20, 2023
1 parent 2d8191a commit c52a29b
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 206 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@salesforce/core",
"version": "4.3.5",
"version": "5.0.0",
"description": "Core libraries to interact with SFDX projects, orgs, and APIs.",
"main": "lib/exported",
"types": "lib/exported.d.ts",
Expand Down
11 changes: 1 addition & 10 deletions src/exported.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,7 @@ export { SfdcUrl } from './util/sfdcUrl';

export { getJwtAudienceUrl } from './util/getJwtAudienceUrl';

export {
Fields,
FieldValue,
LoggerLevel,
LoggerLevelValue,
LogLine,
LoggerOptions,
LoggerStream,
Logger,
} from './logger/logger';
export { Fields, FieldValue, LoggerLevel, LoggerLevelValue, LogLine, LoggerOptions, Logger } from './logger/logger';

export { Messages, StructuredMessage } from './messages';

Expand Down
9 changes: 0 additions & 9 deletions src/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,6 @@ export class Global {
return path.join(os.homedir(), Global.SFDX_STATE_FOLDER);
}

/**
* The full system path to the global log file.
*
* @deprecated. The log file path is now based on a datestamp an not static. This will return incorrect information if used.
*/
// member ordering conflicts with the TS use-before-declaration error
// eslint-disable-next-line @typescript-eslint/member-ordering
public static readonly LOG_FILE_PATH: string = path.join(Global.SF_DIR, 'sf.log');

/**
* Gets the current mode environment variable as a {@link Mode} instance.
*
Expand Down
181 changes: 1 addition & 180 deletions src/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import * as os from 'os';
import * as path from 'path';

import { Writable } from 'stream';
import { Logger as PinoLogger, pino } from 'pino';
import { Env } from '@salesforce/kit';
import { isKeyOf, isString } from '@salesforce/ts-types';
Expand All @@ -17,23 +16,6 @@ import { unwrapArray } from '../util/unwrapArray';
import { MemoryLogger } from './memoryLogger';
import { cleanup } from './cleanup';

/**
* @deprecated. Don't use serializers
* A Bunyan `Serializer` function.
*
* @param input The input to be serialized.
*/
export type Serializer = (input: unknown) => unknown;

/**
* @deprecated. Don't use serializers
* A collection of named `Serializer`s.
*
*/
export interface Serializers {
[key: string]: Serializer;
}

/**
* The common set of `Logger` options.
*/
Expand All @@ -43,38 +25,10 @@ export interface LoggerOptions {
*/
name: string;

/**
* @deprecated. JSON is the only format.
* The logger format type. Current options include LogFmt or JSON (default).
*/
format?: LoggerFormat;

/**
* The logger's serializers.
*
* @deprecated. Serializers are not configurable
*/
serializers?: Serializers;
/**
* Whether or not to log source file, line, and function information.
*
* @deprecated. This never did anything
*/
src?: boolean;
/**
* The desired log level.
*/
level?: LoggerLevelValue;
/**
* @deprecated don't pass in streams. this will no do anything
* A stream to write to.
*/
stream?: Writable;
/**
* @deprecated don't pass in streams. this will no do anything
* An array of streams to write to.
*/
streams?: LoggerStream[];

/**
* Create a logger with the fields set
Expand All @@ -99,47 +53,6 @@ export enum LoggerLevel {
FATAL = 60,
}

/**
* `Logger` format types.
*
* @deprecated JSON is the only option
*/
export enum LoggerFormat {
JSON,
LOGFMT,
}

/**
* A Bunyan stream configuration.
*
* @deprecated. Do not use
*
*/
export interface LoggerStream {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
/**
* The type of stream -- may be inferred from other properties.
*/
type?: string;
/**
* The desired log level for the stream.
*/
level?: LoggerLevelValue;
/**
* The stream to write to. Mutually exclusive with `path`.
*/
stream?: Writable;
/**
* The name of the stream.
*/
name?: string;
/**
* A log file path to write to. Mutually exclusive with `stream`.
*/
path?: string;
}

/**
* Any numeric `Logger` level.
*/
Expand All @@ -152,10 +65,8 @@ export type Fields = Record<string, unknown>;

/**
* All possible field value types.
*
* @deprecated. Fields can be anything, including another object
*/
export type FieldValue = string | number | boolean;
export type FieldValue = string | number | boolean | Fields;

/**
* Log line interface
Expand Down Expand Up @@ -224,18 +135,6 @@ export class Logger {
// The sfdx root logger singleton
private static rootLogger?: Logger;

/**
* @deprecated. Has no effect
*/

public readonly logRotationCount = new Env().getNumber('SF_LOG_ROTATION_COUNT') ?? 2;

/**
* @deprecated. Has no effect, will always be false
* Whether debug is enabled for this Logger.
*/
public debugEnabled = false;

private pinoLogger: PinoLogger;

private memoryLogger?: MemoryLogger;
Expand Down Expand Up @@ -374,36 +273,6 @@ export class Logger {
return Logger.getRoot().pinoLogger;
}

/**
* Adds a stream.
*
* @deprecated. addStream is a no-op
*
* @param stream The stream configuration to add.
* @param defaultLevel The default level of the stream.
*/
// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
public addStream(stream: LoggerStream, defaultLevel?: LoggerLevelValue): void {}

/**
* Adds a file stream to this logger. Resolved or rejected upon completion of the addition.
*
* @deprecated. streams don't change once a logger is built
* @param logFile The path to the log file. If it doesn't exist it will be created.
*/
// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
public async addLogFileStream(logFile: string): Promise<void> {}

/**
* Adds a file stream to this logger. Resolved or rejected upon completion of the addition.
*
* @deprecated. streams don't change once a logger is built
*
* @param logFile The path to the log file. If it doesn't exist it will be created.
*/
// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
public addLogFileStreamSync(logFile: string): void {}

/** get the bare (pino) logger instead of using the class hierarchy */
public getRawLogger(): PinoLogger {
return this.pinoLogger;
Expand Down Expand Up @@ -453,16 +322,6 @@ export class Logger {
return this;
}

/**
* Gets the underlying Bunyan logger.
*
* @deprecated. Bunyan is no longer used. This will return a pino Logger
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
public getBunyanLogger(): any {
return this.pinoLogger;
}

/**
* Compares the requested log level with the current log level. Returns true if
* the requested log level is greater than or equal to the current log level.
Expand All @@ -473,17 +332,6 @@ export class Logger {
return (typeof level === 'string' ? this.pinoLogger.levelVal : level) >= this.getLevel();
}

/**
* @deprecated. Use the useMemoryLogging option when instantiating the logger
* Use in-memory logging for this logger instance instead of any parent streams. Useful for testing.
* For convenience this object is returned.
*
* **WARNING: This cannot be undone for this logger instance.**
*/
public useMemoryLogging(): Logger {
return this;
}

/**
* Gets an array of log line objects. Each element is an object that corresponds to a log line.
*/
Expand Down Expand Up @@ -512,24 +360,6 @@ export class Logger {
}
}

/**
* @deprecated filters are no longer dynamic
* Adds a filter to be applied to all logged messages.
*
* @param filter A function with signature `(...args: any[]) => any[]` that transforms log message arguments.
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function, class-methods-use-this, @typescript-eslint/no-unused-vars
public addFilter(filter: (...args: unknown[]) => unknown): void {}

/**
* @deprecated. No longer necessary
* Close the logger, including any streams, and remove all listeners.
*
* @param fn A function with signature `(stream: LoggerStream) => void` to call for each stream with the stream as an arg.
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this, @typescript-eslint/no-empty-function
public close(fn?: (stream: LoggerStream) => void): void {}

/**
* Create a child logger, typically to add a few log record fields. For convenience this object is returned.
*
Expand Down Expand Up @@ -628,15 +458,6 @@ export class Logger {
this.pinoLogger.fatal(unwrapArray(args));
return this;
}

/**
* @deprecated use the env DEBUG=* or DEBUG=<logger-name> instead
*
* Enables logging to stdout when the DEBUG environment variable is used. It uses the logger
* name as the debug name, so you can do DEBUG=<logger-name> to filter the results to your logger.
*/
// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-empty-function
public enableDEBUG(): void {} /** return various streams that the logger could send data to, depending on the options and env */
}

const getWriteStream = (level = 'warn'): pino.TransportSingleOptions => {
Expand Down
2 changes: 0 additions & 2 deletions src/testSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,8 +691,6 @@ export const restoreContext = (testContext: TestContext): void => {
testContext.configStubs = {};
// Give each test run a clean StateAggregator
StateAggregator.clearInstance();
// get a new clean logger for each run
testContext.TEST_LOGGER.close();
// Allow each test to have their own config aggregator
// @ts-ignore clear for testing.
delete ConfigAggregator.instance;
Expand Down
4 changes: 0 additions & 4 deletions test/unit/loggerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ describe('Logger', () => {

describe('root', () => {
it('should construct the root SF logger', async () => {
$$.SANDBOX.spy(Logger.prototype, 'addFilter');
const defaultLogger = await Logger.root();
expect(defaultLogger).to.be.instanceof(Logger);
expect(defaultLogger.getName()).to.equal('sf');
Expand All @@ -111,15 +110,12 @@ describe('Logger', () => {
});

it('should create the root logger if not already created', async () => {
$$.SANDBOX.stub(Logger.prototype, 'addLogFileStream');
$$.SANDBOX.spy(Logger, 'root');
const rootLogger = await Logger.root();
expect(rootLogger.getName()).to.equal('sf');
expect(await Logger.root()).to.equal(rootLogger);
// @ts-expect-error: called is a sinon spy property
expect(Logger.root['called']).to.be.true;
// @ts-expect-error: called is a sinon spy property
expect(rootLogger.addLogFileStream['called']).to.be.false;
});
});

Expand Down

4 comments on commit c52a29b

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: c52a29b Previous: 7732fb9 Ratio
Child logger creation 451475 ops/sec (±2.86%) 15788 ops/sec (±0.27%) 0.03496982114181295
Logging a string on root logger 512867 ops/sec (±14.36%) 71905 ops/sec (±0.28%) 0.14
Logging an object on root logger 409567 ops/sec (±10.08%) 61909 ops/sec (±0.11%) 0.15
Logging an object with a message on root logger 257819 ops/sec (±12.47%) 29700 ops/sec (±0.19%) 0.12
Logging an object with a redacted prop on root logger 14423 ops/sec (±187.75%) 55283 ops/sec (±0.11%) 3.83
Logging a nested 3-level object on root logger 242267 ops/sec (±14.12%) 50904 ops/sec (±0.19%) 0.21

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - ubuntu-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: c52a29b Previous: 7732fb9 Ratio
Logging an object with a redacted prop on root logger 14423 ops/sec (±187.75%) 55283 ops/sec (±0.11%) 3.83

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - windows-latest

Benchmark suite Current: c52a29b Previous: 7732fb9 Ratio
Child logger creation 598288 ops/sec (±1.27%) 4231 ops/sec (±8.38%) 0.007071844997726847
Logging a string on root logger 817851 ops/sec (±14.13%) 51391 ops/sec (±0.37%) 0.06283662916594832
Logging an object on root logger 479361 ops/sec (±24.56%) 43162 ops/sec (±1.80%) 0.09004070001522861
Logging an object with a message on root logger 244041 ops/sec (±22.60%) 19823 ops/sec (±0.93%) 0.08122815428555039
Logging an object with a redacted prop on root logger 4044 ops/sec (±242.35%) 38869 ops/sec (±0.39%) 9.61
Logging a nested 3-level object on root logger 275780 ops/sec (±15.74%) 33241 ops/sec (±0.63%) 0.12

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - windows-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: c52a29b Previous: 7732fb9 Ratio
Logging an object with a redacted prop on root logger 4044 ops/sec (±242.35%) 38869 ops/sec (±0.39%) 9.61

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.