Subscribe to Rails.event once and use config.log_level in create_default_logger - #61
Merged
Merged
Conversation
Every create_default_logger call subscribes another EventLogSubscriber, so each Rails.event is logged once per call. And the logger always starts at DEBUG unless LOG_LEVEL is set, which a logger added to Rails.logger with broadcast_to after boot keeps, whatever config.log_level says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…log_level EventLogSubscriber.subscribe keeps one subscriber per process, and later create_default_logger calls hand it the new logger instead of adding another subscriber. create_logger, which create_default_logger builds its logger with, sets the level from config.log_level unless LOG_LEVEL is set, so a logger added with broadcast_to after boot no longer logs at debug. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
logtail-rack 0.2.8 times responses with the monotonic clock, which Timecop doesn't freeze, so "Completed 200 OK in 0.0ms" no longer holds. The last CI run on main still resolved logtail-rack 0.2.7. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two problems with
Logtail::Logger.create_default_logger, both reproduced by the red-team on Rails 8.1:EventLogSubscribertoRails.event. An app that calls it twice, e.g. inconfig/application.rband again inconfig/environments/production.rb, sends everyRails.eventrow twice. The red-team's app had twoEventLogSubscribers registered after boot.LOG_LEVELis set. Rails appliesconfig.log_leveltoRails.loggerwhile booting, but a logger added later withRails.logger.broadcast_to(Logtail::Logger.create_default_logger(...))keeps DEBUG and ships debug lines such as SQL queries. In the red-team's appconfig.log_levelwasinfo, butRails.logger.levelwas 0 (debug).What changes:
create_default_loggersubscribes once. Later calls point the existing subscriber at the new logger, soRails.eventrows go to the logger created last, which is the one the app ends up using. This lives in a newEventLogSubscriber.subscribe(logger), and the subscriber'sloggergets a writer.LOG_LEVELenvironment variable if it's set (as before), else toRails.application.config.log_level, else to DEBUG. This is set inLogtail::Logger.create_logger, whichcreate_default_loggerbuilds its logger with, so loggers created directly withcreate_loggerfollowconfig.log_leveltoo.Behaviour and compatibility:
config/application.rbthe level isn't final yet, because environment files may still setconfig.log_level. Rails applies it toRails.loggerin:initialize_logger, so the usual setup behaves as before.Rails.eventrows now.Rails.event, so only the level change applies there.Targets the 0.2.15 patch release. No dependencies.
The commit "Expect real response durations in the request specs" is the same change as in #59: logtail-rack 0.2.8 times responses with the monotonic clock, which Timecop doesn't freeze, so three existing specs now fail on every branch based on main. It's identical in both PRs, so it merges cleanly whichever lands first.
The first commit only adds the tests and is expected to fail on CI. The fix follows in the next commit.
🤖 Generated with Claude Code