Remove CustomLogger?

[shippy]

@adambray replaced the Rails logger with CustomLogger in d62de0918ce085639ea002cb6040bb620e33a26c. Its code seems lifted verbatim from Dennis Reimann, and probably for similar purposes - maybe to make sure that the infamous health check doesn't clutter production logs?

Either way, the logger seems to confuse our better_errors gem and sits on top of the reported stack trace, which hinders development. I vote nix it, but would like to hear opinions (cc: @mnquintana, @dgoerger, @jasonkliu, @njlxyaoxinwei) before I do.

[jasonkliu]

I did a bit of digging on the most recent finish_bootstrap branch.

It seems that we're just ignoring the status controller in the logger? The status controller just returns 'Application is Running', so it's not super helpful: simply loading the front page of the application will have the same effect. Could you elaborate a bit more on the 'infamous health check'?

I think we should remove it for upgrading purposes: overrides of controllers are hard to maintain across different versions of software (devise is a possible exception, where you have to override it for your own functionality). r+

[shippy]

"Infamous health check" is the request every app on Yale servers receives every five seconds to verify that it's still up and running. It is infamous because a Reservation gem nearly crashed the entire Yale RubyFarm.

If that's what it does, then that just seems like a convenience, one that is easily emulated by some grep -v on our log files.

[mnquintana]

I don't see any point in having a status controller at all - we have plenty of other ways of validating that the app is up and running, all of which are better than the status controller. And if all we're getting from the CustomLogger is silencing the status controller, then we should get rid of that too.

[dgoerger]

The Status controller gives a special /status page on the application.

The default F5 health check (the one that runs every five seconds and combined with MiniProfiler on Reservations DEV nearly caused an inodes overload and server trashing [irrecoverably corrupted filesystem]) is just an HTTP check for status 200, i.e., HTTP's "this page exists" message.

We don't need to override this status check with a custom one (status 200 is fine), and the Rails log shouldn't be a problem in terms of inodes because it remains as one file and inodes imposes a limit of how many files can exist. Silencing the F5 health check in the Rails logs has the effect of making the DEV log easier to read (because there won't be a health status log every five seconds), but proper grepping can get around this without too much trouble.

Let me know if you have any more questions on the server side. :)

[mnquintana]

So does this mean that we can safely remove the CustomLogger & Status Controller?

[shippy]

CustomLogger, definitely. StatusController might be a good way to reduce the number of SQL queries per second, though?

[dgoerger]

It helps reduce SQL queries from the F5 health check, yea (no SQL query necessary to print a plaintext string), but SQL queries are nbd afaik. I wouldn't maintain code complexity to lighten SQL queries, since if the SQL queries are slow it's because Shifts is doing them wrong (just like with Reservations, as we discovered this past summer). :)

[dgoerger]

Huh, it turns out the F5 check is satisfied with a 302 redirect-to-CAS, so there's no SQL query nonsense at all! :D