Skip to content

Commit

Permalink
fix: address additional review comments
Browse files Browse the repository at this point in the history
summary of changes
- removed forcedInfo -> startupInfo
- change DiagLogLevel range add STARTUP
- removed explicit numeric values on Core.LogLevel
  • Loading branch information
MSNev committed Feb 9, 2021
1 parent fc00e10 commit a4cb4ba
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 50 deletions.
3 changes: 1 addition & 2 deletions packages/opentelemetry-api/src/api/diag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,8 @@ export class DiagAPI implements DiagLogger {
public debug!: DiagLogFunction;
public info!: DiagLogFunction;
public warn!: DiagLogFunction;
public startupInfo!: DiagLogFunction;
public error!: DiagLogFunction;
public critical!: DiagLogFunction;
public terminal!: DiagLogFunction;

public forcedInfo!: DiagLogFunction;
}
17 changes: 8 additions & 9 deletions packages/opentelemetry-api/src/diag/consoleLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const consoleMap: { n: keyof DiagLogger; c: keyof Console }[] = [
{ n: 'info', c: 'info' },
{ n: 'debug', c: 'debug' },
{ n: 'verbose', c: 'trace' },
{ n: 'forcedInfo', c: 'info' },
{ n: 'startupInfo', c: 'info' },
];

/**
Expand Down Expand Up @@ -73,6 +73,13 @@ export class DiagConsoleLogger implements DiagLogger {
/** Log an error scenario that was not expected and caused the requested operation to fail. */
public error!: DiagLogFunction;

/**
* Logs a general informational message that is used for logging component startup and version
* information without causing additional general informational messages when the logging level
* is set to DiagLogLevel.WARN or lower.
*/
public startupInfo!: DiagLogFunction;

/**
* Log a warning scenario to inform the developer of an issues that should be investigated.
* The requested operation may or may not have succeeded or completed.
Expand Down Expand Up @@ -102,12 +109,4 @@ export class DiagConsoleLogger implements DiagLogger {
* in a production environment.
*/
public verbose!: DiagLogFunction;

/**
* Log a general informational message that should always be logged regardless of the
* current {@Link DiagLogLevel) and configured filtering level. This type of logging is
* useful for logging component startup and version information without causing additional
* general informational messages when the logging level is set to DiagLogLevel.WARN or lower.
*/
public forcedInfo!: DiagLogFunction;
}
34 changes: 18 additions & 16 deletions packages/opentelemetry-api/src/diag/logLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,41 @@ import { DiagLogger, DiagLogFunction, createNoopDiagLogger } from './logger';
* compatibility/migration issues for any implementation that assume the numeric ordering.
*/
export enum DiagLogLevel {
/** DIagnostic Logging level setting to disable all logging (except and forced logs) */
NONE = -99,
/** Diagnostic Logging level setting to disable all logging (except and forced logs) */
NONE = 0,

/**
* Identifies a terminal situation that would cause the API to completely fail to initialize,
* if this type of error is logged functionality of the API is not expected to be functional.
*/
TERMINAL = -2,
TERMINAL = 10,

/**
* Identifies a critical error that needs to be addressed, functionality of the component
* that emits this log detail may non-functional.
*/
CRITICAL = -1,
CRITICAL = 20,

/** Identifies an error scenario */
ERROR = 0,
ERROR = 30,

/** Identifies startup and failure (lower) scenarios */
STARTUP = 40,

/** Identifies a warning scenario */
WARN = 1,
WARN = 50,

/** General informational log message */
INFO = 2,
INFO = 60,

/** General debug log message */
DEBUG = 3,
DEBUG = 70,

/**
* Detailed trace level logging should only be used for development, should only be set
* in a development environment.
*/
VERBOSE = 4,
VERBOSE = 80,

/** Used to set the logging level to include all logging */
ALL = 9999,
Expand All @@ -79,19 +82,19 @@ const fallbackLoggerFuncMap: { [n: string]: keyof Logger } = {
info: 'info',
debug: 'debug',
verbose: 'debug',
forcedInfo: 'info',
startupInfo: 'info',
};

/** Mapping from DiagLogger function name to logging level. */
const levelMap: { n: keyof DiagLogger; l: DiagLogLevel; f?: boolean }[] = [
const levelMap: { n: keyof DiagLogger; l: DiagLogLevel }[] = [
{ n: 'terminal', l: DiagLogLevel.TERMINAL },
{ n: 'critical', l: DiagLogLevel.CRITICAL },
{ n: 'error', l: DiagLogLevel.ERROR },
{ n: 'warn', l: DiagLogLevel.WARN },
{ n: 'info', l: DiagLogLevel.INFO },
{ n: 'debug', l: DiagLogLevel.DEBUG },
{ n: 'verbose', l: DiagLogLevel.VERBOSE },
{ n: 'forcedInfo', l: DiagLogLevel.INFO, f: true },
{ n: 'startupInfo', l: DiagLogLevel.ERROR },
];

/**
Expand Down Expand Up @@ -130,10 +133,9 @@ export function createLogLevelDiagLogger(
function _filterFunc(
theLogger: DiagLogger,
funcName: keyof DiagLogger,
theLevel: DiagLogLevel,
isForced?: boolean
theLevel: DiagLogLevel
): DiagLogFunction {
if (isForced || maxLevel >= theLevel) {
if (maxLevel >= theLevel) {
return function () {
const orgArguments = arguments as unknown;
const theFunc =
Expand All @@ -152,7 +154,7 @@ export function createLogLevelDiagLogger(
const newLogger = {} as DiagLogger;
for (let i = 0; i < levelMap.length; i++) {
const name = levelMap[i].n;
newLogger[name] = _filterFunc(logger, name, levelMap[i].l, levelMap[i].f);
newLogger[name] = _filterFunc(logger, name, levelMap[i].l);
}

return newLogger;
Expand Down
18 changes: 8 additions & 10 deletions packages/opentelemetry-api/src/diag/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ export interface DiagLogger {
/** Log an error scenario that was not expected and caused the requested operation to fail. */
error: DiagLogFunction;

/**
* Logs a general informational message that is used for logging component startup and version
* information without causing additional general informational messages when the logging level
* is set to DiagLogLevel.WARN or lower.
*/
startupInfo: DiagLogFunction;

/**
* Log a warning scenario to inform the developer of an issues that should be investigated.
* The requested operation may or may not have succeeded or completed.
Expand Down Expand Up @@ -82,15 +89,6 @@ export interface DiagLogger {
* in a production environment.
*/
verbose: DiagLogFunction;

/**
* Log a general informational message that should always be logged regardless of the
* current or configured logging level {@Link DiagLogLevel} except when the level is set
* to {@Link DiagLogLevel.NONE). This type of logging is useful for logging component
* startup and version information without causing additional general informational messages
* when the logging level is set to DiagLogLevel.WARN or lower.
*/
forcedInfo: DiagLogFunction;
}

// DiagLogger implementation
Expand All @@ -99,10 +97,10 @@ export const diagLoggerFunctions: Array<keyof DiagLogger> = [
'debug',
'info',
'warn',
'startupInfo',
'error',
'critical',
'terminal',
'forcedInfo',
];

function noopLogFunction() {}
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-api/test/diag/consoleLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const expectedConsoleMap: { [n: string]: keyof Console } = {
info: 'info',
debug: 'debug',
verbose: 'trace',
forcedInfo: 'info',
startupInfo: 'info',
};

describe('DiagConsoleLogger', () => {
Expand Down
30 changes: 23 additions & 7 deletions packages/opentelemetry-api/test/diag/logLevel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const expectedIncompleteMap: { [n: string]: keyof Console } = {
info: 'info',
debug: 'debug',
verbose: 'debug',
forcedInfo: 'info',
startupInfo: 'info',
};

describe('LogLevelFilter DiagLogger', () => {
Expand All @@ -54,7 +54,7 @@ describe('LogLevelFilter DiagLogger', () => {
info: null,
debug: null,
verbose: null,
forcedInfo: null,
startupInfo: null,
};

let dummyLogger: DiagLogger;
Expand Down Expand Up @@ -118,16 +118,31 @@ describe('LogLevelFilter DiagLogger', () => {
{
message: 'CRITICAL',
level: DiagLogLevel.CRITICAL,
ignoreFuncs: ['verbose', 'debug', 'info', 'warn', 'error'],
ignoreFuncs: [
'verbose',
'debug',
'info',
'warn',
'error',
'startupInfo',
],
},
{
message: 'TERMINAL',
level: DiagLogLevel.TERMINAL,
ignoreFuncs: ['verbose', 'debug', 'info', 'warn', 'error', 'critical'],
ignoreFuncs: [
'verbose',
'debug',
'info',
'warn',
'error',
'critical',
'startupInfo',
],
},
{
message: 'between TERMINAL and NONE',
level: -10,
level: 1,
ignoreFuncs: [
'verbose',
'debug',
Expand All @@ -136,6 +151,7 @@ describe('LogLevelFilter DiagLogger', () => {
'error',
'critical',
'terminal',
'startupInfo',
],
},
{
Expand All @@ -149,7 +165,7 @@ describe('LogLevelFilter DiagLogger', () => {
'error',
'critical',
'terminal',
'forcedInfo',
'startupInfo',
],
},
{
Expand All @@ -163,7 +179,7 @@ describe('LogLevelFilter DiagLogger', () => {
'error',
'critical',
'terminal',
'forcedInfo',
'startupInfo',
],
},
];
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-api/test/diag/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('DiagLogger functions', () => {
info: null,
debug: null,
verbose: null,
forcedInfo: null,
startupInfo: null,
};

let dummyLogger: DiagLogger;
Expand Down
8 changes: 4 additions & 4 deletions packages/opentelemetry-core/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import { Exception } from '@opentelemetry/api';
* @see {@link DiagLogLevel} from the api
*/
export enum LogLevel {
ERROR = 0,
WARN = 1,
INFO = 2,
DEBUG = 3,
ERROR,
WARN,
INFO,
DEBUG,
}

/**
Expand Down

0 comments on commit a4cb4ba

Please sign in to comment.