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

Make ActionCable logging less verbose in development #23709

Merged
merged 1 commit into from Feb 19, 2016

Conversation

Projects
None yet
6 participants
@jankeesvw
Contributor

jankeesvw commented Feb 16, 2016

When running the ActionCable server in development I get a lot of log messages, even when I set the application log level to config.log_level = :info. With this commit I move the level for ActionCable to debug which is more appropriate in my opinion. With this commit I truncate the log message as proposed by @dhh in #23709 (comment).

Example of my current output to illustrate the problem:

image

@rails-bot

This comment has been minimized.

Show comment
Hide comment
@rails-bot

rails-bot Feb 16, 2016

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @matthewd (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

rails-bot commented Feb 16, 2016

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @matthewd (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Feb 17, 2016

Member

I think this one make sense to be info since you want to know when the broadcast is being done. Maybe we should only include the message in the debug mode?

r? @dhh

cc @matthewd

Member

rafaelfranca commented Feb 17, 2016

I think this one make sense to be info since you want to know when the broadcast is being done. Maybe we should only include the message in the debug mode?

r? @dhh

cc @matthewd

@rails-bot rails-bot assigned dhh and unassigned matthewd Feb 17, 2016

@jankeesvw

This comment has been minimized.

Show comment
Hide comment
@jankeesvw

jankeesvw Feb 17, 2016

Contributor

I would also be happy to remove the #{message} part. If we remove that it would be a lot more readable for me.

Like this:

server.logger.info "[ActionCable] Broadcasting to #{broadcasting}"
Contributor

jankeesvw commented Feb 17, 2016

I would also be happy to remove the #{message} part. If we remove that it would be a lot more readable for me.

Like this:

server.logger.info "[ActionCable] Broadcasting to #{broadcasting}"
@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Feb 17, 2016

Member

I'd stick with info and just truncate the message part so it doesn't go crazy on a big HTML send. Maybe just do the first 300 chars or so.

Member

dhh commented Feb 17, 2016

I'd stick with info and just truncate the message part so it doesn't go crazy on a big HTML send. Maybe just do the first 300 chars or so.

@jankeesvw jankeesvw changed the title from Set ActionCable logging from info to debug to Make ActionCable logging less verbose in development Feb 17, 2016

@jankeesvw

This comment has been minimized.

Show comment
Hide comment
@jankeesvw

jankeesvw Feb 17, 2016

Contributor

Hi @dhh, I changed the commit as you proposed, I also added three dots (...) so we have a clue this could be shortened. My log now looks like this (which is much better):

PrinterListChannel transmitting "<div class=\"mod-printers-list\" data-id=\"printer-list\">\n  <table class=\"list-view\">\n    <thead>\n      <tr>\n        <th class=\"lead\">\n          Printers\n        </th>\n        <th>\n          Job\n        </th>\n        <th>\n          Time remaining\n        </th>\n        <th colspan=\... (via streamed from PrinterListChannel)
JobListChannel transmitting "<table class=\"list-view mod-jobs-list\" data-id=\"job-list\">\n  <thead>\n    <tr>\n      <th class=\"lead\">\n        Jobs\n      </th>\n      <th>\n        Printer(s)\n      </th>\n      <th>\n        Amount of prints\n      </th>\n      <th>\n        Created at\n      </th>\n    </tr>\n  </thead... (via streamed from JobListChannel)

I would love to hear if this is ready to merge, or if it needs a little more work.

Contributor

jankeesvw commented Feb 17, 2016

Hi @dhh, I changed the commit as you proposed, I also added three dots (...) so we have a clue this could be shortened. My log now looks like this (which is much better):

PrinterListChannel transmitting "<div class=\"mod-printers-list\" data-id=\"printer-list\">\n  <table class=\"list-view\">\n    <thead>\n      <tr>\n        <th class=\"lead\">\n          Printers\n        </th>\n        <th>\n          Job\n        </th>\n        <th>\n          Time remaining\n        </th>\n        <th colspan=\... (via streamed from PrinterListChannel)
JobListChannel transmitting "<table class=\"list-view mod-jobs-list\" data-id=\"job-list\">\n  <thead>\n    <tr>\n      <th class=\"lead\">\n        Jobs\n      </th>\n      <th>\n        Printer(s)\n      </th>\n      <th>\n        Amount of prints\n      </th>\n      <th>\n        Created at\n      </th>\n    </tr>\n  </thead... (via streamed from JobListChannel)

I would love to hear if this is ready to merge, or if it needs a little more work.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Feb 18, 2016

Member

This would give us three dots even when you're not truncating. You can use String#truncate from Active Support to do this.

Member

dhh commented Feb 18, 2016

This would give us three dots even when you're not truncating. You can use String#truncate from Active Support to do this.

@jankeesvw

This comment has been minimized.

Show comment
Hide comment
@jankeesvw

jankeesvw Feb 18, 2016

Contributor

I changed it, now it uses ActiveSupport's truncate method. Is this ready to merge?

Contributor

jankeesvw commented Feb 18, 2016

I changed it, now it uses ActiveSupport's truncate method. Is this ready to merge?

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Feb 18, 2016

Member

You don't actually need to specify the omission part. Three dots is the default. So it can just be truncate(300).

Member

dhh commented Feb 18, 2016

You don't actually need to specify the omission part. Three dots is the default. So it can just be truncate(300).

Truncate ActionCable broadcast message to 300 chars
When running the ActionCable server in development I get a lot of output
in my logs, this commit sets a maximum length of 300 characters for a
broadcast log message.
@jankeesvw

This comment has been minimized.

Show comment
Hide comment
@jankeesvw

jankeesvw Feb 18, 2016

Contributor

Whoops, sorry! Changed!

Contributor

jankeesvw commented Feb 18, 2016

Whoops, sorry! Changed!

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Feb 18, 2016

Member

👌

Member

dhh commented Feb 18, 2016

👌

dhh added a commit that referenced this pull request Feb 19, 2016

Merge pull request #23709 from jankeesvw/set-action-cable-logging-to-…
…debug

Make ActionCable logging less verbose in development

@dhh dhh merged commit 2a83a6e into rails:master Feb 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jankeesvw jankeesvw deleted the jankeesvw:set-action-cable-logging-to-debug branch Feb 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment