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

Only output Server logs in Development #11060

Merged

Conversation

schneems
Copy link
Member

Right now when you start a server via rails s, the logger gets extended so that it logs to the file system and also to stdout. This extension behavior is not "intelligent" and if the default logger is already set to output to stdout, then the contents will be received twice.

To capture logs in accordance with http://www.12factor.net/logs some platforms require the logs to be sent to standard out. If a logger is set to stdout, and the server is started using rails server instead of another method (i.e. thin start etc.) then the app will produce double logs.

This PR fixes the issue by only extending the logger to standard out in the development environment. So that in production you don't get double logs like this:

ActionView::Template::Error (wrong number of arguments (5 for 4)):
    1: <% lang_index = 0 %>
    2: <div class="row">
    3:   <ul class="nav nav-tabs nav-stacked span2" data-tabs="tabs" id="repo-tabs">
    4:     <% repos.group_by(&:language).each do |lang, repos| %>
    5:       <% unless lang == nil %>
    6:         <li><a href="#<%= "#{lang.parameterize}#{lang.hash}" %>" data-toggle="tab"><%= lang %></a></li>
    7:       <% end -%>
  app/views/shared/_repos.html.erb:4:in `_app_views_shared__repos_html_erb___1685450633638247395_70300668607000'
  app/views/pages/index.html.erb:13:in `_app_views_pages_index_html_erb__2084723628308867770_70300687584880'

ActionView::Template::Error (wrong number of arguments (5 for 4)):
    1: <% lang_index = 0 %>
    2: <div class="row">
    3:   <ul class="nav nav-tabs nav-stacked span2" data-tabs="tabs" id="repo-tabs">
    4:     <% repos.group_by(&:language).each do |lang, repos| %>
    5:       <% unless lang == nil %>
    6:         <li><a href="#<%= "#{lang.parameterize}#{lang.hash}" %>" data-toggle="tab"><%= lang %></a></li>
    7:       <% end -%>
  app/views/shared/_repos.html.erb:4:in `_app_views_shared__repos_html_erb___1685450633638247395_70300668607000'
  app/views/pages/index.html.erb:13:in `_app_views_pages_index_html_erb__2084723628308867770_70300687584880'

ATP Railties. Opened against master in favor of #10999

@schneems
Copy link
Member Author

1 week ping cc @drogus

@guilleiguaran
Copy link
Member

I'm assigning to myself for review

@schneems I'll ping you later to rebase this after of my review :)

@rafaelfranca
Copy link
Member

This make sense.

@guilleiguaran
Copy link
Member

@schneems please rebase this. Thanks!!!

Right now when you start a server via `rails s`, the logger gets extended so that it logs to the file system and also to stdout. This extension behavior is not "intelligent" and if the default logger is already set to output to stdout, then the contents will be received twice.

To capture logs in accordance with http://www.12factor.net/logs some platforms require the logs to be sent to standard out. If a logger is set to stdout, and the server is started using `rails server` instead of another method (i.e. `thin start` etc.) then the app will produce double logs.

This PR fixes the issue by only extending the logger to standard out in the development environment. So that in production you don't get double logs like this:

```
ActionView::Template::Error (wrong number of arguments (5 for 4)):
    1: <% lang_index = 0 %>
    2: <div class="row">
    3:   <ul class="nav nav-tabs nav-stacked span2" data-tabs="tabs" id="repo-tabs">
    4:     <% repos.group_by(&:language).each do |lang, repos| %>
    5:       <% unless lang == nil %>
    6:         <li><a href="#<%= "#{lang.parameterize}#{lang.hash}" %>" data-toggle="tab"><%= lang %></a></li>
    7:       <% end -%>
  app/views/shared/_repos.html.erb:4:in `_app_views_shared__repos_html_erb___1685450633638247395_70300668607000'
  app/views/pages/index.html.erb:13:in `_app_views_pages_index_html_erb__2084723628308867770_70300687584880'

ActionView::Template::Error (wrong number of arguments (5 for 4)):
    1: <% lang_index = 0 %>
    2: <div class="row">
    3:   <ul class="nav nav-tabs nav-stacked span2" data-tabs="tabs" id="repo-tabs">
    4:     <% repos.group_by(&:language).each do |lang, repos| %>
    5:       <% unless lang == nil %>
    6:         <li><a href="#<%= "#{lang.parameterize}#{lang.hash}" %>" data-toggle="tab"><%= lang %></a></li>
    7:       <% end -%>
  app/views/shared/_repos.html.erb:4:in `_app_views_shared__repos_html_erb___1685450633638247395_70300668607000'
  app/views/pages/index.html.erb:13:in `_app_views_pages_index_html_erb__2084723628308867770_70300687584880'
```

ATP Railties. Opened against master in favor of rails#10999
@schneems
Copy link
Member Author

Changelog rebase dance done and pushed.

@guilleiguaran
Copy link
Member

Thanks!!!

guilleiguaran added a commit that referenced this pull request Sep 23, 2013
…-master

Only output Server logs in Development
@guilleiguaran guilleiguaran merged commit c21941b into rails:master Sep 23, 2013
@mschulkind
Copy link

While this does solve the problem in non-development environments, the problem still exists in development, so it does not fix issues #10977 or #11415 which were both closed and redirected to this issue.

I like to disable development logging to files since all the do is fill up my filesystem and I watch STDOUT anyway. There still doesn't seem to be a way to disable file logging while maintaining correct logging in non-server processes.

@dougc84
Copy link

dougc84 commented Nov 4, 2013

+1 i can confirm this issue is still present in development.

@guilleiguaran
Copy link
Member

patches are welcome :)

@narzero
Copy link

narzero commented Nov 20, 2013

+1

@mastermike14
Copy link

+1

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.

None yet

7 participants