Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8665 (reopened defect)

Opened 1 year ago

Last modified 3 months ago

[XPATCH] Custom logger ignored by script/server with mongrel

Reported by: wr0ngway Assigned to: bitsweat
Priority: normal Milestone: 1.2.5
Component: Railties Version: edge
Severity: normal Keywords: mongrel logger tiny
Cc:

Description

Setting config.logger in environment.rb has no effect when starting a server with script/server and mongrel. Using “script/server webrick”, or “mongrel_rails start” does work.

I think this is caused by code in the mongrel.rb file that script/server uses which loads just the logging environment (but not my environment) in order to figure out which log file to tail

railties/lib/server/mongrel.rb:

require 'initializer'
Rails::Initializer.run(:initialize_logger)

Loading the environment here causes the RAILS_DEFAULT_LOGGER constant to get set, and thus when my config finally gets loaded when mongrel itself starts up, the config.logger I set gets ignored as the rails init code does nothing if RAILS_DEFAULT_LOGGER is set:

railties/lib/initializer.rb:

def initialize_logger
# if the environment has explicitly defined a logger, use it
return if defined?(RAILS_DEFAULT_LOGGER)
...
end

Attachments

fix_mongrel_logger.diff (418 bytes) - added by josh on 06/18/07 01:42:18.
dont_initialize_logging_before_mongrel_rails_starts.patch (0.6 kB) - added by lazyatom on 08/30/07 10:17:10.
There doesn't seem to be any need to load the initializer, or to initialize logging at this point
remove_logger_init_and_touch_log_before_tailing.diff (1.1 kB) - added by atnan on 02/28/08 04:56:05.
Removing logger initialization from Mongrel startup, and ensuring the the log file exists before tailing.

Change History

06/18/07 01:42:18 changed by josh

  • attachment fix_mongrel_logger.diff added.

06/18/07 01:43:21 changed by josh

  • keywords changed from script server mongrel config logger to mongrel logger tiny.
  • owner changed from core to bitsweat.
  • summary changed from Custom logger ignored by script/server with mongrel to [PATCH] Custom logger ignored by script/server with mongrel.

Pretty simple fix.

08/30/07 10:16:28 changed by lazyatom

I can confirm this as a bug (see also http://nubyonrails.com/articles/a-hodel-3000-compliant-logger-for-the-rest-of-us).

An alternative fix is to simply remove loading of the initializer and logger here - it doesn't seem to serve any purpose.

Verified! Please apply either of these patches!

08/30/07 10:17:10 changed by lazyatom

  • attachment dont_initialize_logging_before_mongrel_rails_starts.patch added.

There doesn't seem to be any need to load the initializer, or to initialize logging at this point

08/30/07 10:20:34 changed by lazyatom

This also seems to relate to #7084, which additionally adds some patching to the log path, but I can't say whether or not that's relevant to this bug.

08/30/07 10:43:27 changed by lifofifo

  • keywords changed from mongrel logger tiny to mongrel logger tiny verified.

+1

(follow-up: ↓ 6 ) 08/30/07 15:59:57 changed by wr0ngway

Actually, the first patch (fix_mongrel_logger.diff ) is no good - while it does fix the bug, it introduces another one - the environment gets sourced multiple times. e.g. set a constant in environment.rb and you'll see a warning that it gets set twice. I haven't tried the second patch yet.

(in reply to: ↑ 5 ) 09/02/07 23:34:58 changed by nzkoz

  • keywords changed from mongrel logger tiny verified to mongrel logger tiny.
  • summary changed from [PATCH] Custom logger ignored by script/server with mongrel to [XPATCH] Custom logger ignored by script/server with mongrel.

Replying to wr0ngway:

Actually, the first patch (fix_mongrel_logger.diff ) is no good - while it does fix the bug, it introduces another one - the environment gets sourced multiple times. e.g. set a constant in environment.rb and you'll see a warning that it gets set twice. I haven't tried the second patch yet.

Removing verified flag until these comments are addressed.

09/04/07 18:54:26 changed by emil

It looks that the reason to initialize logger at this point is so that the following code can work.

tail_thread = tail(Pathname.new("#{File.expand_path(RAILS_ROOT)}/log/#{RAILS_ENV}.log").cleanpath)

If the Pathname does not exist the ENOENT is raised. This clearly comes up if one uses some logging rotation strategy (log4r doing development_20070831.log etc) and the default log file is then missing. Perhaps mongrel startup is doing more then what it should with that tail_thread.

09/12/07 15:26:10 changed by stevemidgley

I'd love to see this issue fixed as well. +1

10/15/07 08:40:03 changed by bitsweat

  • status changed from new to closed.
  • resolution set to incomplete.

No forward progress here -- anyone have a definitive solution?

11/07/07 22:46:34 changed by vo.x

  • status changed from closed to reopened.
  • resolution deleted.

As far as I can say, the initialization of logger is completely unnecessary. It is just some sanity attempt to create the log file if it does not exists. If the configuration should be followed, then there need to be used some approach like proposed in #7084, or use code:

log_path = Pathname.new("#{File.expand_path(RAILS_ROOT)}/log/#{RAILS_ENV}.log").cleanpath File.open(log_path, "w") if !File.exist?(log_path)

instead of Rails::Initializer.run(:initialize_logger). It is not the best solution, because it is not following the configuration, but at least it is not more harmful then the current state.

Anyway I believe that tail should be either removed or optional.

11/08/07 09:40:23 changed by vo.x

I have to review my fix again. As I stated before, the introduction of tail was completely bad option. This even prevent the log from rotating if desired. Could you please consider to remove the tailing from script server as there are other options as open new terminal with tail command, or introduce the script/tails if desired? Should I create new ticket for this?

02/28/08 04:56:05 changed by atnan

  • attachment remove_logger_init_and_touch_log_before_tailing.diff added.

Removing logger initialization from Mongrel startup, and ensuring the the log file exists before tailing.

02/28/08 05:01:38 changed by atnan

@bitsweat: I've attached a patch which seems to work fine. The only minor issue is that Mongrel tails the log file when it starts up, which means that it needs to exist (Errno::ENOENT). It doesn't seem like the right place to be creating the log file, but I don't think it's *too* messy.

-- Nathan de Vries

04/29/08 18:25:01 changed by wr0ngway

The solution in the patch remove_logger_init_and_touch_log_before_tailing works for me as well - can someone apply it?