getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.95k stars 1.56k forks source link

Add ability to customize the default logger #9210

Open DCzajkowski opened 1 year ago

DCzajkowski commented 1 year ago

Problem Statement

Currently, the default logger used by Sentry is globalThis.console. However, our app has multiple transports for logging (remote, file, console etc.)

With current Sentry implementation, we cannot redirect default Sentry logs to any of our transports without modifying the global console object.

Solution Brainstorm

We suggest adding a new configuration option that allows to specify the logging module, the same way you can specify a transport.

I would expect LoggerConsoleMethods to be exported as a common interface to adhere to. The logger configuration option would be:

// When specified, used in place of the global `console.log`.
logger?: LoggerConsoleMethods
mydea commented 1 year ago

Hey,

thanks for writing in. I think it may make sense to allow to configure this, however we'd need to build this in a way that does not rely on globals (e.g. the custom logger needs to be on the client, and the logger util needs to check this), and does not increase complexity/bundle size too much. We have a lot of stuff on our plate right now, so I'm gonna put this in the backlog, but PRs are welcome if you've got time to look into this!

DCzajkowski commented 1 year ago

Previously, Sentry used globalThis.__SENTRY__.logger. We realize it was an internal thing, but we overrode it like this:

const nodeGlobal = global as typeof globalThis & { __SENTRY__: { logger: unknown } }

nodeGlobal.__SENTRY__ = nodeGlobal.__SENTRY__ || {}
nodeGlobal.__SENTRY__.logger = new (class {
  constructor(private enabled = false) {}

  disable() {
    this.enabled = false
  }

  enable() {
    this.enabled = true
  }

  log(...args: unknown[]) {
    // logger used here is our internal logger that supports multiple transports
    this.enabled && logger.debug(...args)
  }

  warn(...args: unknown[]) {
    this.enabled && logger.warn(...args)
  }

  error(...args: unknown[]) {
    this.enabled && logger.error(...args)
  }
})()

With the new release, this broke, because Sentry does not use the logger in __SENTRY__ object anymore.

lforst commented 1 year ago

@DCzajkowski yeah we do not recommend relying on this object. It is internal API and does not follow semver.

keinsell commented 8 months ago

+1 on this, would be useful for projects with a little more advanced logging mechanics than console.

lforst commented 8 months ago

This is on our backlog but still not on our immediate roadmap. Now would be the perfect time to contribute as we are preparing a new major release, meaning you could even break some APIs and get away with it!