swift-log icon indicating copy to clipboard operation
swift-log copied to clipboard

Publicify system's default factory

Open MahdiBM opened this issue 2 years ago • 12 comments

Exposes the logging system's default factory to the public.

Motivation:

I've been using a Discord logger for my own application, and I was thinking of making it public so others can use it as well. How it works is that i provide the Logger with a custom LogHandler which is just a skin for the 2 underlying log handlers. The first of the two, is just a normal handler that outputs to the stdout, and the second one, is my custom Discord log handler that sends the logs to Discord. There are multiple reasons to use another log handler besides Discord.

  • You can't fully-send too-long logs to Discord.
  • There might be a network issue.
  • Discord might just 400 your request.
  • There are ratelimits that need to be respected.

So when trying to clean things up for making it public, i figured i can use the system's default log handler as the main handler instead of just hard-coding a handler of my choice. This is unfortunately not possible (or at least in a clean-way) since the default log handler is not accessible from either LoggingSystem.factory() or Logger().handler.

Modifications:

Changed the LoggingSystem's get-only static var factory from fileprivate to public .

Result:

Custom implementations will be able to use the LoggingSystem's factory too.

MahdiBM avatar Jul 05 '22 15:07 MahdiBM

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Jul 05 '22 15:07 swift-server-bot

I take this PR back 😅

The problem came from the fact that I thought Vapor automatically bootstraps the logging system on Application init and doesn't let you stop it from doing so, and then I thought if Vapor's doing it, others would probably do it as well. But a few days ago I found out it's in fact just a line of code in Vapor's templates that does the bootstrap, and Vapor users can just remove that line. Not sure how i missed that line of code considering I really was looking for a solution ...

The fact that you can stop Vapor from bootstrapping, leaves this PR will little purpose. What I was trying to achieve was to be able to grab the log handler that Vapor configures, so something like a Discord Logger could pick the log handler and use it in a multiplex log handler. Then you could for example do DiscordLogger.giveMeALogger() and it would automatically give you a logger that still uses the default-configured log handler, but also logs to Discord.

But now I realize considering that I can ask users to bootstrap the logging system with my custom logger implementation, I can also ask them to pass a log handler of their choice as the main stdout logger. This way users can just do Logger() and they'll receive a Discord logger.

So long story short, I think this PR doesn't serve enough purpose and should be closed.

MahdiBM avatar Jan 24 '23 06:01 MahdiBM

Cool, glad you found that stray bootstrap call :-)

Thanks for writing up your findings, that all makes sense -- let's close this PR.

ktoso avatar Feb 01 '23 05:02 ktoso