From 3a4cf2b076a8f04f54b6edff4f661f5c3b2c8157 Mon Sep 17 00:00:00 2001 From: Justin Menga Date: Mon, 15 May 2017 10:28:18 +1200 Subject: [PATCH] Code review changes Signed-off-by: Justin Menga --- daemon/logger/awslogs/cloudwatchlogs.go | 15 ++++++++--- daemon/logger/awslogs/cloudwatchlogs_test.go | 28 ++++++++++---------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/daemon/logger/awslogs/cloudwatchlogs.go b/daemon/logger/awslogs/cloudwatchlogs.go index 304da2faf2..818b7bdfc3 100644 --- a/daemon/logger/awslogs/cloudwatchlogs.go +++ b/daemon/logger/awslogs/cloudwatchlogs.go @@ -367,7 +367,14 @@ var newTicker = func(freq time.Duration) *time.Ticker { // maximumBytesPerPut). Log messages are split by the maximum bytes per event // (defined in maximumBytesPerEvent). There is a fixed per-event byte overhead // (defined in perEventBytes) which is accounted for in split- and batch- -// calculations. +// calculations. If the awslogs-multiline-pattern or awslogs-datetime-format +// options have been configured, multiline processing is enabled, where +// log messages are stored in an event buffer until a multiline pattern match +// is found, at which point the messages in the event buffer are pushed to +// CloudWatch logs as a single log event. Multline messages still are processed +// according to the maximumBytesPerPut constraint, and the implementation only +// allows for messages to be buffered for a maximum of 2*batchPublishFrequency +// seconds. func (l *logStream) collectBatch() { timer := newTicker(batchPublishFrequency) var events []wrappedEvent @@ -414,10 +421,10 @@ func (l *logStream) collectBatch() { processedLine := append(unprocessedLine, "\n"...) eventBuffer = append(eventBuffer, processedLine...) logger.PutMessage(msg) - continue + } else { + events = l.processEvent(events, unprocessedLine, msg.Timestamp.UnixNano()/int64(time.Millisecond)) + logger.PutMessage(msg) } - events = l.processEvent(events, unprocessedLine, msg.Timestamp.UnixNano()/int64(time.Millisecond)) - logger.PutMessage(msg) } } } diff --git a/daemon/logger/awslogs/cloudwatchlogs_test.go b/daemon/logger/awslogs/cloudwatchlogs_test.go index 541b8d8c99..655b99c7a4 100644 --- a/daemon/logger/awslogs/cloudwatchlogs_test.go +++ b/daemon/logger/awslogs/cloudwatchlogs_test.go @@ -533,16 +533,16 @@ func TestCollectBatchMultilinePattern(t *testing.T) { // Verify single multiline event argument := <-mockClient.putLogEventsArgument assert.NotNil(t, argument, "Expected non-nil PutLogEventsInput") - assert.Equal(t, 1, len(argument.LogEvents), "Expected LogEvents to contain 1 elements, but contains %d", len(argument.LogEvents)) - assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Expected message to be %s but was %s", logline+logline, *argument.LogEvents[0].Message) + assert.Equal(t, 1, len(argument.LogEvents), "Expected single multiline event") + assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Received incorrect multiline message") stream.Close() // Verify single event argument = <-mockClient.putLogEventsArgument assert.NotNil(t, argument, "Expected non-nil PutLogEventsInput") - assert.Equal(t, 1, len(argument.LogEvents), "Expected LogEvents to contain 1 elements, but contains %d", len(argument.LogEvents)) - assert.Equal(t, "xxxx "+logline+"\n", *argument.LogEvents[0].Message, "Expected message to be %s but was %s", "xxxx "+logline, *argument.LogEvents[0].Message) + assert.Equal(t, 1, len(argument.LogEvents), "Expected single multiline event") + assert.Equal(t, "xxxx "+logline+"\n", *argument.LogEvents[0].Message, "Received incorrect multiline message") } func BenchmarkCollectBatch(b *testing.B) { @@ -646,8 +646,8 @@ func TestCollectBatchMultilinePatternMaxEventAge(t *testing.T) { // Verify single multiline event is flushed after maximum event buffer age (batchPublishFrequency) argument := <-mockClient.putLogEventsArgument assert.NotNil(t, argument, "Expected non-nil PutLogEventsInput") - assert.Equal(t, 1, len(argument.LogEvents), "Expected LogEvents to contain 1 elements, but contains %d", len(argument.LogEvents)) - assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Expected message to be %s but was %s", logline+logline, *argument.LogEvents[0].Message) + assert.Equal(t, 1, len(argument.LogEvents), "Expected single multiline event") + assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Received incorrect multiline message") stream.Close() } @@ -694,8 +694,8 @@ func TestCollectBatchMultilinePatternNegativeEventAge(t *testing.T) { // Verify single multiline event is flushed with a negative event buffer age argument := <-mockClient.putLogEventsArgument assert.NotNil(t, argument, "Expected non-nil PutLogEventsInput") - assert.Equal(t, 1, len(argument.LogEvents), "Expected LogEvents to contain 1 elements, but contains %d", len(argument.LogEvents)) - assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Expected message to be %s but was %s", logline+logline, *argument.LogEvents[0].Message) + assert.Equal(t, 1, len(argument.LogEvents), "Expected single multiline event") + assert.Equal(t, logline+"\n"+logline+"\n", *argument.LogEvents[0].Message, "Received incorrect multiline message") stream.Close() } @@ -961,8 +961,8 @@ func TestParseLogOptionsMultilinePattern(t *testing.T) { } multilinePattern, err := parseMultilineOptions(info) - assert.Nil(t, err, "Received unexpected err: %v\n", err) - assert.True(t, multilinePattern.MatchString("xxxx"), "Expected multilinePattern to match string xxxx but no match found") + assert.Nil(t, err, "Received unexpected error") + assert.True(t, multilinePattern.MatchString("xxxx"), "No multiline pattern match found") } func TestParseLogOptionsDatetimeFormat(t *testing.T) { @@ -986,8 +986,8 @@ func TestParseLogOptionsDatetimeFormat(t *testing.T) { }, } multilinePattern, err := parseMultilineOptions(info) - assert.Nil(t, err, "Received unexpected err: %v\n", err) - assert.True(t, multilinePattern.MatchString(dt.match), "Expected multilinePattern %s to match string %s but no match found", dt.format, dt.match) + assert.Nil(t, err, "Received unexpected error") + assert.True(t, multilinePattern.MatchString(dt.match), "No multiline pattern match found") }) } } @@ -1001,8 +1001,8 @@ func TestValidateLogOptionsDatetimeFormatAndMultilinePattern(t *testing.T) { conflictingLogOptionsError := "you cannot configure log opt 'awslogs-datetime-format' and 'awslogs-multiline-pattern' at the same time" err := ValidateLogOpt(cfg) - assert.NotNil(t, err, "Expected an error but received nil") - assert.Equal(t, err.Error(), conflictingLogOptionsError, "Received incorrect error: %v\n", err) + assert.NotNil(t, err, "Expected an error") + assert.Equal(t, err.Error(), conflictingLogOptionsError, "Received invalid error") } func TestCreateTagSuccess(t *testing.T) {