Truncating log messages when they are too large #315

juliancheal · 2017-11-21T18:21:50Z

No description provided.

agrare · 2017-11-21T18:29:14Z

lib/gems/pending/util/vmdb-logger.rb

@@ -157,7 +157,7 @@ class Formatter < Logger::Formatter

    def call(severity, time, progname, msg)
      msg = prefix_task_id(msg2str(msg))
-
+      msg = truncate_large_lines(msg)


How about just msg = prefix_task_id(msg2str(msg)).truncate(max_log_line_length) ?

Guess so, was worried about making an incomplete char at the end of the line, then again that could happen with byteslice too.

And I guess we're truncating the line anyway so a bust char wouldn't be the end of the world?

yeah and it automatically appends ... so you can tell that it was truncated

Fryguy · 2017-11-21T21:18:59Z

lib/gems/pending/util/vmdb-logger.rb

@@ -156,8 +156,9 @@ class Formatter < Logger::Formatter
    FORMAT = "[----] %s, [%s#%d:%x] %5s -- %s: %s\n"

    def call(severity, time, progname, msg)
-      msg = prefix_task_id(msg2str(msg))
+      max_log_line_length = 1.megabyte # TODO: (julian) Move to config


Don't worry about config, because I can't see a user configuring this.

Ok thanks @Fryguy.

You might need to add some activesupport extensions at the top of this file...it can run standalone without the rails env. See how we use core_ext above.

Fryguy · 2017-11-21T21:20:35Z

spec/util/vmdb-logger_spec.rb

@@ -95,6 +95,18 @@
    expect(VMDBLogger.contents("mylog.log")).to eq("")
  end

+  context "long messages" do
+    before(:each) do


The :each is redundant, so you can remove it... however, you can also just make this a let

let(:logger) { VMDBLogger.new(@log) }

See way up above for an example of using let

I was future proofing for when we add more tests :)

Fryguy · 2017-11-21T21:21:14Z

I think my other comment got lost;

You might need to add some activesupport extensions at the top of this file...it can run standalone without the rails env. See how we use core_ext above.

juliancheal · 2017-11-22T14:36:58Z

🍏 ❓

agrare · 2017-11-22T14:55:42Z

spec/util/vmdb-logger_spec.rb

@@ -1,5 +1,6 @@
 require 'util/vmdb-logger'
 require 'util/miq-password'
+require 'active_support/core_ext/string'


Think you need this in lib/gems/pending/util/vmdb-logger.rb too

Preferably in the vmdb-logger and not in the spec.

Oh I see, so active_support/core_ext/string should be included via util/vmdb-logger? @Fryguy

Yes, since that's where you actually use it (for the truncate call)

Fryguy · 2017-11-27T15:36:28Z

lib/gems/pending/util/vmdb-logger.rb

@@ -156,8 +157,9 @@ class Formatter < Logger::Formatter
    FORMAT = "[----] %s, [%s#%d:%x] %5s -- %s: %s\n"

    def call(severity, time, progname, msg)
-      msg = prefix_task_id(msg2str(msg))
+      max_log_line_length = 1.megabyte


Since this call will happen very frequently, I'm thinking this might be better moved to a constant. That way it doesn't have to re-evaluate 1.megabyte over and over.

Also, is there an active_support/core_ext for this megabyte method?

Good point it is.

juliancheal · 2017-11-28T14:46:01Z

@Fryguy changes all done.

Allows the tests and gems-pending code to be run standalone. Changed before(:each) to simple let(:foo) in specs MAX_LOG_LINE_LENGTH is a constant

juliancheal · 2017-12-06T15:43:49Z

@Fryguy I thought I'd pushed my changes, guess not. Sorry my bad! Just made the change you asked for. Should hopefully be green.

miq-bot · 2017-12-06T15:48:44Z

Checked commits juliancheal/manageiq-gems-pending@ad4fb78~...3f16029 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

juliancheal · 2017-12-12T21:16:24Z

@Fryguy 🍏 We good to go?

Truncating log messages when they are too large (cherry picked from commit d09f86a)

simaishi · 2018-01-04T23:16:18Z

Gaprindashvili backport details:

$ git log -1
commit 36602cedbf23f88d9e7d8e78d7517e70e36ed227
Author: Jason Frey <[email protected]>
Date:   Thu Jan 4 10:33:47 2018 -0500

    Merge pull request #315 from juliancheal/fix_logger_size_limit
    
    Truncating log messages when they are too large
    (cherry picked from commit d09f86aa78fdaf68b832f28c69cd8024df416bca)

miq-bot assigned Fryguy Nov 21, 2017

miq-bot added the wip label Nov 21, 2017

agrare reviewed Nov 21, 2017

View reviewed changes

juliancheal force-pushed the fix_logger_size_limit branch 2 times, most recently from bb7c84b to b65291e Compare November 21, 2017 18:56

Truncating log messages when they are too large

ad4fb78

juliancheal force-pushed the fix_logger_size_limit branch 3 times, most recently from 9be4917 to 7cfac18 Compare November 21, 2017 21:06

juliancheal changed the title ~~[WIP] Truncating log messages when they are too large~~ Truncating log messages when they are too large Nov 21, 2017

juliancheal force-pushed the fix_logger_size_limit branch from 7cfac18 to 4bd5c1d Compare November 21, 2017 21:08

miq-bot removed the wip label Nov 21, 2017

juliancheal force-pushed the fix_logger_size_limit branch 2 times, most recently from 3971069 to 673ed3d Compare November 21, 2017 21:15

Fryguy reviewed Nov 21, 2017

View reviewed changes

Added spec test to check msg is truncated to 1.megabyte

921f694

juliancheal force-pushed the fix_logger_size_limit branch from 673ed3d to 921f694 Compare November 21, 2017 21:19

Fryguy reviewed Nov 21, 2017

View reviewed changes

agrare reviewed Nov 22, 2017

View reviewed changes

juliancheal force-pushed the fix_logger_size_limit branch from 24b2d56 to 3eb11a5 Compare November 22, 2017 14:58

Fryguy reviewed Nov 27, 2017

View reviewed changes

juliancheal force-pushed the fix_logger_size_limit branch from 3eb11a5 to 23a2664 Compare November 28, 2017 14:39

juliancheal force-pushed the fix_logger_size_limit branch from 23a2664 to 33af821 Compare November 28, 2017 14:49

Added ActiveSupport#String

3f16029

Allows the tests and gems-pending code to be run standalone. Changed before(:each) to simple let(:foo) in specs MAX_LOG_LINE_LENGTH is a constant

juliancheal force-pushed the fix_logger_size_limit branch from 33af821 to 3f16029 Compare December 6, 2017 15:42

Fryguy merged commit d09f86a into ManageIQ:master Jan 4, 2018

Fryguy added gaprindashvili/yes bug labels Jan 4, 2018

Fryguy added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 4, 2018

simaishi pushed a commit that referenced this pull request Jan 4, 2018

Merge pull request #315 from juliancheal/fix_logger_size_limit

36602ce

Truncating log messages when they are too large (cherry picked from commit d09f86a)

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jan 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Truncating log messages when they are too large #315

Truncating log messages when they are too large #315

juliancheal commented Nov 21, 2017

agrare Nov 21, 2017

juliancheal Nov 21, 2017

agrare Nov 21, 2017

Fryguy Nov 21, 2017

juliancheal Nov 21, 2017

Fryguy Nov 21, 2017

Fryguy Nov 21, 2017

Fryguy Nov 21, 2017

juliancheal Nov 21, 2017

Fryguy commented Nov 21, 2017

juliancheal commented Nov 22, 2017

agrare Nov 22, 2017

Fryguy Nov 27, 2017

juliancheal Nov 27, 2017

Fryguy Dec 4, 2017

Fryguy Nov 27, 2017

Fryguy Nov 27, 2017 •

edited

Loading

juliancheal Nov 27, 2017

juliancheal commented Nov 28, 2017

juliancheal commented Dec 6, 2017

miq-bot commented Dec 6, 2017

juliancheal commented Dec 12, 2017

simaishi commented Jan 4, 2018

Truncating log messages when they are too large #315

Truncating log messages when they are too large #315

Conversation

juliancheal commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy commented Nov 21, 2017

juliancheal commented Nov 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliancheal commented Nov 28, 2017

juliancheal commented Dec 6, 2017

miq-bot commented Dec 6, 2017

juliancheal commented Dec 12, 2017

simaishi commented Jan 4, 2018

Fryguy Nov 27, 2017 •

edited

Loading