Logger improvements (#7729)

* Remove enablement

* Refactor context and filtering

* Fix imports, simplify transports config

* Migrate usages of debug context

* Re-org, add colors and grouping to console logging

* Remove temp default context

* Remove manual prefix

* Move colorizing out of console transport body

* Reduce reuse

* Pass through context

* Ensure bitdrift is enabled in dev

* Enable Sentry on web only

* Clean up types

* Docs

* Format

* Update tests

* Clean up tests

* No positional args

* Revert Sentry changes

* Clean up context, use it, pass metadata through to Bitdrift

* Fix up debugging

* Clean up metadata before passing to Bitdrift

* Correct transports

* Reserve context prop on metadata and include in transports

* Update tests

src/logger/__tests__/logger.test.ts

@@ -2,16 +2,9 @@ import {beforeAll, describe, expect, jest, test} from '@jest/globals'
 import * as Sentry from '@sentry/react-native'
 import {nanoid} from 'nanoid/non-secure'
 
-import {Logger, LogLevel, sentryTransport} from '#/logger'
-
-jest.mock('#/env', () => ({
-  /*
-   * Forces debug mode for tests using the default logger. Most tests create
-   * their own logger instance.
-   */
-  LOG_LEVEL: 'debug',
-  LOG_DEBUG: '',
-}))
+import {Logger} from '#/logger'
+import {sentryTransport} from '#/logger/transports/sentry'
+import {LogLevel} from '#/logger/types'
 
 jest.mock('@sentry/react-native', () => ({
   addBreadcrumb: jest.fn(),
@@ -26,57 +19,27 @@ beforeAll(() => {
 describe('general functionality', () => {
   test('default params', () => {
     const logger = new Logger()
-    expect(logger.enabled).toBeFalsy()
-    expect(logger.level).toEqual(LogLevel.Debug) // mocked above
+    expect(logger.level).toEqual(LogLevel.Info)
   })
 
   test('can override default params', () => {
     const logger = new Logger({
-      enabled: true,
+      level: LogLevel.Debug,
+    })
+    expect(logger.level).toEqual(LogLevel.Debug)
+  })
+
+  test('contextFilter overrides level', () => {
+    const logger = new Logger({
       level: LogLevel.Info,
+      contextFilter: 'test',
     })
-    expect(logger.enabled).toBeTruthy()
-    expect(logger.level).toEqual(LogLevel.Info)
-  })
-
-  test('disabled logger does not report', () => {
-    const logger = new Logger({
-      enabled: false,
-      level: LogLevel.Debug,
-    })
-
-    const mockTransport = jest.fn()
-
-    logger.addTransport(mockTransport)
-    logger.debug('message')
-
-    expect(mockTransport).not.toHaveBeenCalled()
-  })
-
-  test('disablement', () => {
-    const logger = new Logger({
-      enabled: true,
-      level: LogLevel.Debug,
-    })
-
-    logger.disable()
-
-    const mockTransport = jest.fn()
-
-    logger.addTransport(mockTransport)
-    logger.debug('message')
-
-    expect(mockTransport).not.toHaveBeenCalled()
-  })
-
-  test('passing debug contexts automatically enables debug mode', () => {
-    const logger = new Logger({debug: 'specific'})
     expect(logger.level).toEqual(LogLevel.Debug)
   })
 
   test('supports extra metadata', () => {
     const timestamp = Date.now()
-    const logger = new Logger({enabled: true})
+    const logger = new Logger({})
 
     const mockTransport = jest.fn()
 
@@ -87,6 +50,7 @@ describe('general functionality', () => {
 
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       'message',
       extra,
       timestamp,
@@ -95,7 +59,7 @@ describe('general functionality', () => {
 
   test('supports nullish/falsy metadata', () => {
     const timestamp = Date.now()
-    const logger = new Logger({enabled: true})
+    const logger = new Logger({})
 
     const mockTransport = jest.fn()
 
@@ -105,6 +69,7 @@ describe('general functionality', () => {
     logger.warn('a', null)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       'a',
       {},
       timestamp,
@@ -114,6 +79,7 @@ describe('general functionality', () => {
     logger.warn('b', false)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       'b',
       {},
       timestamp,
@@ -123,6 +89,7 @@ describe('general functionality', () => {
     logger.warn('c', 0)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       'c',
       {},
       timestamp,
@@ -130,7 +97,7 @@ describe('general functionality', () => {
 
     remove()
 
-    logger.addTransport((level, message, metadata) => {
+    logger.addTransport((level, context, message, metadata) => {
       expect(typeof metadata).toEqual('object')
     })
 
@@ -143,10 +110,17 @@ describe('general functionality', () => {
     const timestamp = Date.now()
     const sentryTimestamp = timestamp / 1000
 
-    sentryTransport(LogLevel.Debug, message, {}, timestamp)
-    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+    sentryTransport(
+      LogLevel.Debug,
+      Logger.Context.Default,
       message,
-      data: {},
+      {},
+      timestamp,
+    )
+    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+      category: Logger.Context.Default,
+      message,
+      data: {context: 'logger'},
       type: 'default',
       level: LogLevel.Debug,
       timestamp: sentryTimestamp,
@@ -154,22 +128,31 @@ describe('general functionality', () => {
 
     sentryTransport(
       LogLevel.Info,
+      Logger.Context.Default,
       message,
       {type: 'info', prop: true},
       timestamp,
     )
     expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+      category: Logger.Context.Default,
       message,
-      data: {prop: true},
+      data: {prop: true, context: 'logger'},
       type: 'info',
       level: LogLevel.Info,
       timestamp: sentryTimestamp,
     })
 
-    sentryTransport(LogLevel.Log, message, {}, timestamp)
-    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+    sentryTransport(
+      LogLevel.Log,
+      Logger.Context.Default,
       message,
-      data: {},
+      {},
+      timestamp,
+    )
+    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+      category: Logger.Context.Default,
+      message,
+      data: {context: 'logger'},
       type: 'default',
       level: 'debug', // Sentry bug, log becomes debug
       timestamp: sentryTimestamp,
@@ -177,14 +160,21 @@ describe('general functionality', () => {
     jest.runAllTimers()
     expect(Sentry.captureMessage).toHaveBeenCalledWith(message, {
       level: 'log',
-      tags: undefined,
-      extra: {},
+      tags: {category: 'logger'},
+      extra: {context: 'logger'},
     })
 
-    sentryTransport(LogLevel.Warn, message, {}, timestamp)
-    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+    sentryTransport(
+      LogLevel.Warn,
+      Logger.Context.Default,
       message,
-      data: {},
+      {},
+      timestamp,
+    )
+    expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({
+      category: Logger.Context.Default,
+      message,
+      data: {context: 'logger'},
       type: 'default',
       level: 'warning',
       timestamp: sentryTimestamp,
@@ -192,8 +182,8 @@ describe('general functionality', () => {
     jest.runAllTimers()
     expect(Sentry.captureMessage).toHaveBeenCalledWith(message, {
       level: 'warning',
-      tags: undefined,
-      extra: {},
+      tags: {category: 'logger'},
+      extra: {context: 'logger'},
     })
 
     const e = new Error('error')
@@ -203,6 +193,7 @@ describe('general functionality', () => {
 
     sentryTransport(
       LogLevel.Error,
+      Logger.Context.Default,
       e,
       {
         tags,
@@ -212,9 +203,13 @@ describe('general functionality', () => {
     )
 
     expect(Sentry.captureException).toHaveBeenCalledWith(e, {
-      tags,
+      tags: {
+        ...tags,
+        category: 'logger',
+      },
       extra: {
         prop: true,
+        context: 'logger',
       },
     })
   })
@@ -226,6 +221,7 @@ describe('general functionality', () => {
 
     sentryTransport(
       LogLevel.Debug,
+      undefined,
       message,
       {error: new Error('foo')},
       timestamp,
@@ -241,7 +237,7 @@ describe('general functionality', () => {
 
   test('add/remove transport', () => {
     const timestamp = Date.now()
-    const logger = new Logger({enabled: true})
+    const logger = new Logger({})
     const mockTransport = jest.fn()
 
     const remove = logger.addTransport(mockTransport)
@@ -256,6 +252,7 @@ describe('general functionality', () => {
     expect(mockTransport).toHaveBeenNthCalledWith(
       1,
       LogLevel.Warn,
+      undefined,
       'warn',
       {},
       timestamp,
@@ -263,22 +260,43 @@ describe('general functionality', () => {
   })
 })
 
-describe('debug contexts', () => {
-  const mockTransport = jest.fn()
+describe('create', () => {
+  test('create', () => {
+    const mockTransport = jest.fn()
+    const timestamp = Date.now()
+    const message = nanoid()
+    const logger = Logger.create(Logger.Context.Default)
 
+    logger.addTransport(mockTransport)
+    logger.info(message, {})
+
+    expect(mockTransport).toHaveBeenCalledWith(
+      LogLevel.Info,
+      Logger.Context.Default,
+      message,
+      {},
+      timestamp,
+    )
+  })
+})
+
+describe('debug contexts', () => {
   test('specific', () => {
+    const mockTransport = jest.fn()
     const timestamp = Date.now()
     const message = nanoid()
     const logger = new Logger({
-      enabled: true,
-      debug: 'specific',
+      // @ts-ignore
+      context: 'specific',
+      level: LogLevel.Debug,
     })
 
     logger.addTransport(mockTransport)
-    logger.debug(message, {}, 'specific')
+    logger.debug(message, {})
 
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Debug,
+      'specific',
       message,
       {},
       timestamp,
@@ -286,18 +304,22 @@ describe('debug contexts', () => {
   })
 
   test('namespaced', () => {
+    const mockTransport = jest.fn()
     const timestamp = Date.now()
     const message = nanoid()
     const logger = new Logger({
-      enabled: true,
-      debug: 'namespace*',
+      // @ts-ignore
+      context: 'namespace:foo',
+      contextFilter: 'namespace:*',
+      level: LogLevel.Debug,
     })
 
     logger.addTransport(mockTransport)
-    logger.debug(message, {}, 'namespace')
+    logger.debug(message, {})
 
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Debug,
+      'namespace:foo',
       message,
       {},
       timestamp,
@@ -305,18 +327,21 @@ describe('debug contexts', () => {
   })
 
   test('ignores inactive', () => {
+    const mockTransport = jest.fn()
     const timestamp = Date.now()
     const message = nanoid()
     const logger = new Logger({
-      enabled: true,
-      debug: 'namespace:foo:*',
+      // @ts-ignore
+      context: 'namespace:bar:baz',
+      contextFilter: 'namespace:foo:*',
     })
 
     logger.addTransport(mockTransport)
-    logger.debug(message, {}, 'namespace:bar:baz')
+    logger.debug(message, {})
 
     expect(mockTransport).not.toHaveBeenCalledWith(
       LogLevel.Debug,
+      'namespace:bar:baz',
       message,
       {},
       timestamp,
@@ -328,7 +353,6 @@ describe('supports levels', () => {
   test('debug', () => {
     const timestamp = Date.now()
     const logger = new Logger({
-      enabled: true,
       level: LogLevel.Debug,
     })
     const message = nanoid()
@@ -339,6 +363,7 @@ describe('supports levels', () => {
     logger.debug(message)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Debug,
+      undefined,
       message,
       {},
       timestamp,
@@ -347,6 +372,7 @@ describe('supports levels', () => {
     logger.info(message)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Info,
+      undefined,
       message,
       {},
       timestamp,
@@ -355,6 +381,7 @@ describe('supports levels', () => {
     logger.warn(message)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       message,
       {},
       timestamp,
@@ -362,13 +389,18 @@ describe('supports levels', () => {
 
     const e = new Error(message)
     logger.error(e)
-    expect(mockTransport).toHaveBeenCalledWith(LogLevel.Error, e, {}, timestamp)
+    expect(mockTransport).toHaveBeenCalledWith(
+      LogLevel.Error,
+      undefined,
+      e,
+      {},
+      timestamp,
+    )
   })
 
   test('info', () => {
     const timestamp = Date.now()
     const logger = new Logger({
-      enabled: true,
       level: LogLevel.Info,
     })
     const message = nanoid()
@@ -382,6 +414,7 @@ describe('supports levels', () => {
     logger.info(message)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Info,
+      undefined,
       message,
       {},
       timestamp,
@@ -391,7 +424,6 @@ describe('supports levels', () => {
   test('warn', () => {
     const timestamp = Date.now()
     const logger = new Logger({
-      enabled: true,
       level: LogLevel.Warn,
     })
     const message = nanoid()
@@ -408,6 +440,7 @@ describe('supports levels', () => {
     logger.warn(message)
     expect(mockTransport).toHaveBeenCalledWith(
       LogLevel.Warn,
+      undefined,
       message,
       {},
       timestamp,
@@ -417,7 +450,6 @@ describe('supports levels', () => {
   test('error', () => {
     const timestamp = Date.now()
     const logger = new Logger({
-      enabled: true,
       level: LogLevel.Error,
     })
     const message = nanoid()
@@ -436,6 +468,12 @@ describe('supports levels', () => {
 
     const e = new Error('original message')
     logger.error(e)
-    expect(mockTransport).toHaveBeenCalledWith(LogLevel.Error, e, {}, timestamp)
+    expect(mockTransport).toHaveBeenCalledWith(
+      LogLevel.Error,
+      undefined,
+      e,
+      {},
+      timestamp,
+    )
   })
 })