feat(diag-logger): part 2 - breaking changes - remove api.Logger, api… (open-telemetry#1925)

Author: MSNev

api/src/api/diag.ts

@@ -99,9 +99,9 @@ export class DiagAPI implements DiagLogger {
       return _logger;
     };
 
-    self.setLogger = (logger: DiagLogger): DiagLogger => {
+    self.setLogger = (logger?: DiagLogger): DiagLogger => {
       const prevLogger = _logger;
-      if (prevLogger !== logger && logger !== self) {
+      if (!logger || logger !== self) {
         // Simple special case to avoid any possible infinite recursion on the logging functions
         _logger = logger || createNoopDiagLogger();
         _filteredLogger = createLogLevelDiagLogger(_logLevel, _logger);
@@ -133,10 +133,10 @@ export class DiagAPI implements DiagLogger {
 
   /**
    * Set the DiagLogger instance
-   * @param logger - The DiagLogger instance to set as the default logger
+   * @param logger - [Optional] The DiagLogger instance to set as the default logger, if not provided it will set it back as a noop
    * @returns The previously registered DiagLogger
    */
-  public setLogger!: (logger: DiagLogger) => DiagLogger;
+  public setLogger!: (logger?: DiagLogger) => DiagLogger;
 
   /** Set the default maximum diagnostic logging level */
   public setLogLevel!: (maxLogLevel: DiagLogLevel) => void;
@@ -146,8 +146,5 @@ 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;
 }

api/src/common/Logger.ts

@@ -17,20 +17,17 @@
 import { DiagLogger, DiagLogFunction } from './logger';
 
 const consoleMap: { n: keyof DiagLogger; c: keyof Console }[] = [
-  { n: 'terminal', c: 'error' },
-  { n: 'critical', c: 'error' },
   { n: 'error', c: 'error' },
   { n: 'warn', c: 'warn' },
   { n: 'info', c: 'info' },
   { n: 'debug', c: 'debug' },
   { n: 'verbose', c: 'trace' },
-  { n: 'startupInfo', c: 'info' },
 ];
 
 /**
  * A simple Immutable Console based diagnostic logger which will output any messages to the Console.
  * If you want to limit the amount of logging to a specific level or lower use the
- * {@link diagLogLevelFilter}
+ * {@link createLogLevelDiagLogger}
  */
 export class DiagConsoleLogger implements DiagLogger {
   constructor() {
@@ -58,28 +55,9 @@ export class DiagConsoleLogger implements DiagLogger {
     }
   }
 
-  /**
-   * Log a terminal situation that would cause the API to completely fail to initialize,
-   * if this type of message is logged functionality of the API is not expected to be functional.
-   */
-  public terminal!: DiagLogFunction;
-
-  /**
-   * Log a critical error that NEEDS to be addressed, functionality of the component that emits
-   * this log detail may non-functional. While the overall API may be.
-   */
-  public critical!: DiagLogFunction;
-
   /** 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.

api/src/diag/consoleLogger.ts

@@ -15,7 +15,6 @@
  */
 
 import { DiagAPI } from '../api/diag';
-import { Logger } from '../common/Logger';
 import { DiagLogger, DiagLogFunction, createNoopDiagLogger } from './logger';
 
 /**
@@ -27,24 +26,9 @@ export enum DiagLogLevel {
   /** 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 = 10,
-
-  /**
-   * Identifies a critical error that needs to be addressed, functionality of the component
-   * that emits this log detail may non-functional.
-   */
-  CRITICAL = 20,
-
   /** Identifies an error scenario */
   ERROR = 30,
 
-  /** Identifies startup and failure (lower) scenarios */
-  STARTUP = 40,
-
   /** Identifies a warning scenario */
   WARN = 50,
 
@@ -66,35 +50,17 @@ export enum DiagLogLevel {
 
 /**
  * This is equivalent to:
- * type LogLevelString = 'NONE' | TERMINAL' | 'CRITICAL' | 'ERROR' | 'WARN' | 'INFO' | 'DEBUG' | 'VERBOSE' | 'ALL';
+ * type LogLevelString = 'NONE' | 'ERROR' | 'WARN' | 'INFO' | 'DEBUG' | 'VERBOSE' | 'ALL';
  */
 export type DiagLogLevelString = keyof typeof DiagLogLevel;
 
-/**
- * Mapping from DiagLogger function name to Legacy Logger function used if
- * the logger instance doesn't have the DiagLogger function
- */
-const fallbackLoggerFuncMap: { [n: string]: keyof Logger } = {
-  terminal: 'error',
-  critical: 'error',
-  error: 'error',
-  warn: 'warn',
-  info: 'info',
-  debug: 'debug',
-  verbose: 'debug',
-  startupInfo: 'info',
-};
-
 /** Mapping from DiagLogger function name to logging level. */
 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: 'startupInfo', l: DiagLogLevel.ERROR },
 ];
 
 /**
@@ -138,8 +104,7 @@ export function createLogLevelDiagLogger(
     if (maxLevel >= theLevel) {
       return function () {
         const orgArguments = arguments as unknown;
-        const theFunc =
-          theLogger[funcName] || theLogger[fallbackLoggerFuncMap[funcName]];
+        const theFunc = theLogger[funcName];
         if (theFunc && typeof theFunc === 'function') {
           return theFunc.apply(
             logger,

api/src/diag/logLevel.ts

@@ -14,52 +14,20 @@
  * limitations under the License.
  */
 
-import { Logger } from '../common/Logger';
-
-/**
- * Defines a type which can be used for as a parameter without breaking backward
- * compatibility. The {@link Logger} reference will be removed with the removal
- * of the Logger definition, this can be used as a replacement for functions
- * that are currently passing Logger references during migration to minimize
- * breaks that will occur with the removal of the Logger interface.
- */
-export type OptionalDiagLogger = Logger | DiagLogger | null | undefined;
-
 export type DiagLogFunction = (message: string, ...args: unknown[]) => void;
 
 /**
  * Defines an internal diagnostic logger interface which is used to log internal diagnostic
  * messages, you can set the default diagnostic logger via the {@link DiagAPI} setLogger function.
  * API provided implementations include :-
  * - a No-Op {@link createNoopDiagLogger}
- * - a {@link DiagLogLevel} filtering wrapper {@link diagLogLevelFilter}
+ * - a {@link DiagLogLevel} filtering wrapper {@link createLogLevelDiagLogger}
  * - a general Console {@link DiagConsoleLogger} version.
  */
 export interface DiagLogger {
-  /**
-   * Log a terminal situation that would cause the API to completely fail to initialize,
-   * if this type of message is logged functionality of the API is not expected to be functional.
-   */
-  terminal: DiagLogFunction;
-
-  /**
-   * Log a critical error that NEEDS to be addressed, functionality of the component that emits
-   * this log detail may be limited or non-functional depending on when this message is emitted.
-   * Unlike terminal message, it is expected that the overall API may still be functional, again
-   * depending on what component and when this message is emitted.
-   */
-  critical: DiagLogFunction;
-
   /** 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.
@@ -97,10 +65,7 @@ export const diagLoggerFunctions: Array<keyof DiagLogger> = [
   'debug',
   'info',
   'warn',
-  'startupInfo',
   'error',
-  'critical',
-  'terminal',
 ];
 
 function noopLogFunction() {}

api/src/diag/logger.ts

@@ -15,7 +15,6 @@
  */
 
 export * from './common/Exception';
-export * from './common/Logger';
 export * from './common/Time';
 export * from './context/context';
 export * from './context/propagation/TextMapPropagator';
@@ -25,7 +24,6 @@ export * from './trace/attributes';
 export * from './trace/Event';
 export * from './trace/link_context';
 export * from './trace/link';
-export * from './trace/NoopLogger';
 export * from './trace/NoopTracer';
 export * from './trace/NoopTracerProvider';
 export * from './trace/ProxyTracer';

api/src/index.ts

@@ -31,7 +31,11 @@ import api, {
   ROOT_CONTEXT,
   defaultTextMapSetter,
   defaultTextMapGetter,
+  diag,
+  diagLoggerFunctions,
 } from '../../src';
+import { DiagAPI } from '../../src/api/diag';
+import { _global } from '../../src/api/global-utils';
 import { NoopSpan } from '../../src/trace/NoopSpan';
 
 describe('API', () => {
@@ -177,4 +181,42 @@ describe('API', () => {
       });
     });
   });
+
+  describe('Global diag', () => {
+    it('initialization', () => {
+      const inst = DiagAPI.instance();
+
+      assert.deepStrictEqual(diag, inst);
+    });
+
+    diagLoggerFunctions.forEach(fName => {
+      it(`no argument logger ${fName} message doesn't throw`, () => {
+        diag.setLogger();
+        assert.doesNotThrow(() => {
+          diag[fName](`${fName} message`);
+        });
+      });
+
+      it(`null logger ${fName} message doesn't throw`, () => {
+        diag.setLogger(null as any);
+        assert.doesNotThrow(() => {
+          diag[fName](`${fName} message`);
+        });
+      });
+
+      it(`undefined logger ${fName} message doesn't throw`, () => {
+        diag.setLogger(undefined as any);
+        assert.doesNotThrow(() => {
+          diag[fName](`${fName} message`);
+        });
+      });
+
+      it(`empty logger ${fName} message doesn't throw`, () => {
+        diag.setLogger({} as any);
+        assert.doesNotThrow(() => {
+          diag[fName](`${fName} message`);
+        });
+      });
+    });
+  });
 });

api/src/trace/NoopLogger.ts

@@ -28,14 +28,11 @@ const consoleFuncs: Array<keyof Console> = [
 ];
 
 const expectedConsoleMap: { [n: string]: keyof Console } = {
-  terminal: 'error',
-  critical: 'error',
   error: 'error',
   warn: 'warn',
   info: 'info',
   debug: 'debug',
   verbose: 'trace',
-  startupInfo: 'info',
 };
 
 describe('DiagConsoleLogger', () => {