Replace logger with sdk_logger

[solnic]

This is in preparation for #2604 - we have an internal Sentry.logger used for warn/error/debug logging with the SDK but with Structured Logging feature we're introducing a public-facing Sentry.logger API, so our internal one needs to be renamed.

[st0012]

Not blocking: Although we marked it as private in docs, it's still very accessible to users. So I'd suggest keeping it like:

def logger
  print_deprecation_message_to_ask_users_not_to_use_it
  configuration.sdk_logger
end

Then at least some users of it can have time to move off of it.

[solnic]

@st0012 so, this is tricky - in #2600 we're adding Sentry.logger interface, so it is not going to be deprecated, but it will be enabled conditionally based on config._experiments[:enable_logs]. What we could do is printing deprecation message only if enabled_logs is not true - WDYT? The message would say something like "Starting with 5.{version_where_we_enable_logging_by_default} Sentry.logger will become public API for structured logging feature"

[sl0thentr0py]

sorry late to the convo, we can also go for Sentry::Logger with module level methods instead of Sentry.logger if we want to keep the old one unchanged..

[solnic]

@sl0thentr0py I think there's no way around that - IF there are people who use Sentry.logger, then we gotta let them know via deprecation warning that this logger is changing its purpose. I'm not sure how changing to Sentry::Logger class methods would help?

[sl0thentr0py]

we still deprecate but all the new stuff would go to Sentry:: Logger and Sentry.logger would stay what it was for now till next major

[solnic]

@sl0thentr0py oh I see. So instead of config.sdk_logger we switch to global Sentry::Logger and deprecate original config.logger behavior. Yeah this makes perfect sense. It would simplify LoggingHelper and a bunch of constructors. One question though - in sentry-rails we actually set the logger to a duped Rails.logger, what would happen with this if we want to switch to Sentry::Logger and remove configurable logger?

[sl0thentr0py]

ok let me be clearer,

about the internal SDK logging:

• config.logger stays but deprecated
• config.sdk_logger comes and replaces config.logger

--

Now, everything about the new Sentry Logging Feature goes under a completely new namespace Sentry::Logging.do_something (not Sentry::Logger since that's also taken) and NOT Sentry.logger.do_something

[solnic]

Now, everything about the new Sentry Logging Feature goes under a completely new namespace Sentry::Logging.do_something (not Sentry::Logger since that's also taken) and NOT Sentry.logger.do_something

I don't think it's a good idea as it would diverge from the naming convention from other SDKs where we already have Sentry.logger - also, it's just much nicer and intuitive vs Sentry::Logging.

What would be the benefit of going with Sentry::Logging? The way I see it is that we just need to deal with the deprecation cycle of config.logger / Sentry.logger being used as an internal logging facility and eventually replace it with the new logging stuff.

Please remember that initially we are going to release new logging with _experiments[:enable_logger] config option, so the users will have to make a deliberate choice to enable it and we can simply add docs/release notes + maybe even extra warning on init, that say that when this is enabled, then config.logger / Sentry.logger is no longer the same thing and if the user relies on it, then they should basically stop doing that as it was always meant to be used solely as an internal SDK logger.

[sl0thentr0py]

resolved in slack, we will show deprecation and do the switch based on the config

[solnic]

@st0012 @sl0thentr0py I re-added Sentry.logger method via cb55544 and it has a deprecation warning now: