Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the Ability to be Able to Configure the Logger #772

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BenMorganMY
Copy link

@BenMorganMY BenMorganMY commented Jul 25, 2024

This PR allows Shoryuken to be able to configure the logger such as:

Shoryuken.logger = Appsignal::Logger.new('shoryuken')
Shoryuken.logger.formatter = Shoryuken::Logging::WithoutTimestamp.new

This provides some feature parity to Sidekiq and helps developers that are using a service like AppSignal get their logs up there.

https://docs.appsignal.com/logging/platforms/integrations/ruby.html#sidekiq-logger

@DanHenton
Copy link

Just passing by, however, I've been using a custom logger for years with no issue.

You can already configure the Shoryuken logger via the options field.

# disclaimer this is sudo code and untested...
require "logger"

class MyCustomLogger < Logger; end

custom_logger = MyCustomLogger.new("my_custom_logger")

Shoryuken.options[:logger] = custom_logger

Or in the config options hash you can pass in the custom_logger instance.

Shoryuken will now log via your custom logger where you can redefine your implementation.

@BenMorganMY
Copy link
Author

BenMorganMY commented Aug 7, 2024

@DanHenton I believe I began with that and wanted to take it further. I wasn't confident that Shoryuken would stick with the configuration that was set via the options. I wanted a first class option so that the logger never accidentally goes to the default log.

I also needed a log that would be compatible with AppSignal for usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants