Skip to content

Change JrubyRackLogger to Logger #260

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

Closed

Conversation

jlahtinen
Copy link
Contributor

I am not sure if Logger is always loaded or not. With rails app it seems it is.

I am not sure if Logger is always loaded or not. With rails app it seems it is.
@jlahtinen
Copy link
Contributor Author

I can not say exactly why this initializes Rails.logger in a way it works with requests that has a session management and silence_logger is called.

For some reason silence_logger is not added to JRuby::Rack::Logger but it is added to ::Logger

@headius
Copy link
Member

headius commented Jun 20, 2024

@kares Got a sec to look this over? This was a necessary change to get @jlahtinen's app running with jruby-rack 1.2.x.

@headius headius added this to the 1.2.3 milestone Jun 20, 2024
@kares
Copy link
Member

kares commented Aug 8, 2024

So this kind of restores the 1.1 behavior. Which is not the intent here, the idea was to ship with a custom JRuby-Rack logger impl by default. The reason being having better Ruby logging -> Java logging backend integration. Which was not that good in 1.1.x I think e.g. mapping ruby logging level properly to the Java one (would need to look deeper).

For the record, users could use their custom config.logger configuration in Rails, which is respected (and this code won't be used if they set that in Rails).

For some reason silence_logger is not added to JRuby::Rack::Logger but it is added to ::Logger

Would be nice to have a proper fix, the methods missing but required by recent Rails should be added to the JRuby::Rack::Logger impl

@jlahtinen
Copy link
Contributor Author

Thanks for the update. I try to investigate this further.

@jlahtinen
Copy link
Contributor Author

This problem rose because the usage of activerecord-session_store version that had its own silence_logger implementation. In our case it was activerecord-session_store 1.1.3.

This problem is gone when using latest activerecord-session_store. I think that is ok and this can be closed.

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