Split StreamConfig from New, Utest table driven

Signed-off-by: Maximiliano Maccanti <maccanti@amazon.com>

daemon/logger/awslogs/cloudwatchlogs.go

@@ -81,6 +81,16 @@ type logStream struct {
 	sequenceToken      *string
 }
 
+type logStreamConfig struct {
+	logStreamName      string
+	logGroupName       string
+	logCreateGroup     bool
+	logNonBlocking     bool
+	forceFlushInterval time.Duration
+	maxBufferedEvents  int
+	multilinePattern   *regexp.Regexp
+}
+
 var _ logger.SizedLogger = &logStream{}
 
 type api interface {
@@ -128,65 +138,28 @@ type eventBatch struct {
 // AWS_REGION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, the shared credentials
 // file (~/.aws/credentials), and the EC2 Instance Metadata Service.
 func New(info logger.Info) (logger.Logger, error) {
-	logGroupName := info.Config[logGroupKey]
-	logStreamName, err := loggerutils.ParseLogTag(info, "{{.FullID}}")
-	if err != nil {
-		return nil, err
-	}
-	logCreateGroup := false
-	if info.Config[logCreateGroupKey] != "" {
-		logCreateGroup, err = strconv.ParseBool(info.Config[logCreateGroupKey])
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	logNonBlocking := info.Config["mode"] == "non-blocking"
-
-	forceFlushInterval := defaultForceFlushInterval
-	if info.Config[forceFlushIntervalKey] != "" {
-		forceFlushIntervalAsInt, err := strconv.Atoi(info.Config[forceFlushIntervalKey])
-		if err != nil {
-			return nil, err
-		}
-		forceFlushInterval = time.Duration(forceFlushIntervalAsInt) * time.Second
-	}
-
-	maxBufferedEvents := int(defaultMaxBufferedEvents)
-	if info.Config[maxBufferedEventsKey] != "" {
-		maxBufferedEvents, err = strconv.Atoi(info.Config[maxBufferedEventsKey])
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	if info.Config[logStreamKey] != "" {
-		logStreamName = info.Config[logStreamKey]
-	}
-
-	multilinePattern, err := parseMultilineOptions(info)
+	containerStreamConfig, err := newStreamConfig(info)
 	if err != nil {
 		return nil, err
 	}
-
 	client, err := newAWSLogsClient(info)
 	if err != nil {
 		return nil, err
 	}
 
 	containerStream := &logStream{
-		logStreamName:      logStreamName,
-		logGroupName:       logGroupName,
-		logCreateGroup:     logCreateGroup,
-		logNonBlocking:     logNonBlocking,
-		forceFlushInterval: forceFlushInterval,
-		multilinePattern:   multilinePattern,
+		logStreamName:      containerStreamConfig.logStreamName,
+		logGroupName:       containerStreamConfig.logGroupName,
+		logCreateGroup:     containerStreamConfig.logCreateGroup,
+		logNonBlocking:     containerStreamConfig.logNonBlocking,
+		forceFlushInterval: containerStreamConfig.forceFlushInterval,
+		multilinePattern:   containerStreamConfig.multilinePattern,
 		client:             client,
-		messages:           make(chan *logger.Message, maxBufferedEvents),
+		messages:           make(chan *logger.Message, containerStreamConfig.maxBufferedEvents),
 	}
 
 	creationDone := make(chan bool)
-	if logNonBlocking {
+	if containerStream.logNonBlocking {
 		go func() {
 			backoff := 1
 			maxBackoff := 32
@@ -226,6 +199,63 @@ func New(info logger.Info) (logger.Logger, error) {
 	return containerStream, nil
 }
 
+// Parses most of the awslogs- options and prepares a config object to be used for newing the actual stream
+// It has been formed out to ease Utest of the New above
+func newStreamConfig(info logger.Info) (*logStreamConfig, error) {
+	logGroupName := info.Config[logGroupKey]
+	logStreamName, err := loggerutils.ParseLogTag(info, "{{.FullID}}")
+	if err != nil {
+		return nil, err
+	}
+	logCreateGroup := false
+	if info.Config[logCreateGroupKey] != "" {
+		logCreateGroup, err = strconv.ParseBool(info.Config[logCreateGroupKey])
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	logNonBlocking := info.Config["mode"] == "non-blocking"
+
+	forceFlushInterval := defaultForceFlushInterval
+	if info.Config[forceFlushIntervalKey] != "" {
+		forceFlushIntervalAsInt, err := strconv.Atoi(info.Config[forceFlushIntervalKey])
+		if err != nil {
+			return nil, err
+		}
+		forceFlushInterval = time.Duration(forceFlushIntervalAsInt) * time.Second
+	}
+
+	maxBufferedEvents := int(defaultMaxBufferedEvents)
+	if info.Config[maxBufferedEventsKey] != "" {
+		maxBufferedEvents, err = strconv.Atoi(info.Config[maxBufferedEventsKey])
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if info.Config[logStreamKey] != "" {
+		logStreamName = info.Config[logStreamKey]
+	}
+
+	multilinePattern, err := parseMultilineOptions(info)
+	if err != nil {
+		return nil, err
+	}
+
+	containerStreamConfig := &logStreamConfig{
+		logStreamName:      logStreamName,
+		logGroupName:       logGroupName,
+		logCreateGroup:     logCreateGroup,
+		logNonBlocking:     logNonBlocking,
+		forceFlushInterval: forceFlushInterval,
+		maxBufferedEvents:  maxBufferedEvents,
+		multilinePattern:   multilinePattern,
+	}
+
+	return containerStreamConfig, nil
+}
+
 // Parses awslogs-multiline-pattern and awslogs-datetime-format options
 // If awslogs-datetime-format is present, convert the format from strftime
 // to regexp and return.

daemon/logger/awslogs/cloudwatchlogs_test.go

@@ -10,6 +10,7 @@ import (
 	"reflect"
 	"regexp"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -59,6 +60,59 @@ func testEventBatch(events []wrappedEvent) *eventBatch {
 	return batch
 }
 
+func TestNewStreamConfig(t *testing.T) {
+	tests := []struct {
+		logStreamName      string
+		logGroupName       string
+		logCreateGroup     string
+		logNonBlocking     string
+		forceFlushInterval string
+		maxBufferedEvents  string
+		datetimeFormat     string
+		multilinePattern   string
+		shouldErr          bool
+		testName           string
+	}{
+		{"", groupName, "", "", "", "", "", "", false, "defaults"},
+		{"", groupName, "invalid create group", "", "", "", "", "", true, "invalid create group"},
+		{"", groupName, "", "", "invalid flush interval", "", "", "", true, "invalid flush interval"},
+		{"", groupName, "", "", "", "invalid max buffered events", "", "", true, "invalid max buffered events"},
+		{"", groupName, "", "", "", "", "", "n{1001}", true, "invalid multiline pattern"},
+		{"", groupName, "", "", "15", "", "", "", true, "flush interval at 15"},
+		{"", groupName, "", "", "", "1024", "", "", true, "max buffered events at 1024"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.testName, func(t *testing.T) {
+			cfg := map[string]string{
+				logGroupKey:           tc.logGroupName,
+				logCreateGroupKey:     tc.logCreateGroup,
+				"mode":                tc.logNonBlocking,
+				forceFlushIntervalKey: tc.forceFlushInterval,
+				maxBufferedEventsKey:  tc.maxBufferedEvents,
+				logStreamKey:          tc.logStreamName,
+				datetimeFormatKey:     tc.datetimeFormat,
+				multilinePatternKey:   tc.multilinePattern,
+			}
+
+			info := logger.Info{
+				Config: cfg,
+			}
+			logStreamConfig, err := newStreamConfig(info)
+			if tc.shouldErr {
+				assert.Check(t, err != nil, "Expected an error")
+			} else {
+				assert.Check(t, err == nil, "Unexpected error")
+				assert.Check(t, logStreamConfig.logGroupName == tc.logGroupName, "Unexpected logGroupName")
+				if tc.forceFlushInterval != "" {
+					forceFlushIntervalAsInt, _ := strconv.Atoi(info.Config[forceFlushIntervalKey])
+					assert.Check(t, logStreamConfig.forceFlushInterval == time.Duration(forceFlushIntervalAsInt)*time.Second, "Unexpected forceFlushInterval")
+				}
+			}
+		})
+	}
+}
+
 func TestNewAWSLogsClientUserAgentHandler(t *testing.T) {
 	info := logger.Info{
 		Config: map[string]string{
@@ -1420,65 +1474,63 @@ func TestValidateLogOptionsDatetimeFormatAndMultilinePattern(t *testing.T) {
 }
 
 func TestValidateLogOptionsForceFlushIntervalSeconds(t *testing.T) {
-	cfg := map[string]string{
-		forceFlushIntervalKey: "0",
-		logGroupKey:           groupName,
+	tests := []struct {
+		input     string
+		shouldErr bool
+	}{
+		{"0", true},
+		{"-1", true},
+		{"a", true},
+		{"10", false},
 	}
-	nonPositiveIntegerLogOptionsError := "must specify a positive integer for log opt 'awslogs-force-flush-interval-seconds': 0"
-
-	err := ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
 
-	cfg[forceFlushIntervalKey] = "-1"
-	nonPositiveIntegerLogOptionsError = "must specify a positive integer for log opt 'awslogs-force-flush-interval-seconds': -1"
-
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
-
-	cfg[forceFlushIntervalKey] = "a"
-	nonPositiveIntegerLogOptionsError = "must specify a positive integer for log opt 'awslogs-force-flush-interval-seconds': a"
-
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
-
-	cfg[forceFlushIntervalKey] = "10"
+	for _, tc := range tests {
+		t.Run(tc.input, func(t *testing.T) {
+			cfg := map[string]string{
+				forceFlushIntervalKey: tc.input,
+				logGroupKey:           groupName,
+			}
 
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err == nil, "Unexpected error")
+			err := ValidateLogOpt(cfg)
+			if tc.shouldErr {
+				expectedErr := "must specify a positive integer for log opt 'awslogs-force-flush-interval-seconds': " + tc.input
+				assert.Check(t, err != nil, "Expected an error")
+				assert.Check(t, is.Equal(err.Error(), expectedErr), "Received invalid error")
+			} else {
+				assert.Check(t, err == nil, "Unexpected error")
+			}
+		})
+	}
 }
 
 func TestValidateLogOptionsMaxBufferedEvents(t *testing.T) {
-	cfg := map[string]string{
-		maxBufferedEventsKey: "0",
-		logGroupKey:          groupName,
+	tests := []struct {
+		input     string
+		shouldErr bool
+	}{
+		{"0", true},
+		{"-1", true},
+		{"a", true},
+		{"10", false},
 	}
-	nonPositiveIntegerLogOptionsError := "must specify a positive integer for log opt 'awslogs-max-buffered-events': 0"
-
-	err := ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
 
-	cfg[maxBufferedEventsKey] = "-1"
-	nonPositiveIntegerLogOptionsError = "must specify a positive integer for log opt 'awslogs-max-buffered-events': -1"
-
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
-
-	cfg[maxBufferedEventsKey] = "a"
-	nonPositiveIntegerLogOptionsError = "must specify a positive integer for log opt 'awslogs-max-buffered-events': a"
-
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err != nil, "Expected an error")
-	assert.Check(t, is.Equal(err.Error(), nonPositiveIntegerLogOptionsError), "Received invalid error")
-
-	cfg[maxBufferedEventsKey] = "10"
+	for _, tc := range tests {
+		t.Run(tc.input, func(t *testing.T) {
+			cfg := map[string]string{
+				maxBufferedEventsKey: tc.input,
+				logGroupKey:          groupName,
+			}
 
-	err = ValidateLogOpt(cfg)
-	assert.Check(t, err == nil, "Unexpected error")
+			err := ValidateLogOpt(cfg)
+			if tc.shouldErr {
+				expectedErr := "must specify a positive integer for log opt 'awslogs-max-buffered-events': " + tc.input
+				assert.Check(t, err != nil, "Expected an error")
+				assert.Check(t, is.Equal(err.Error(), expectedErr), "Received invalid error")
+			} else {
+				assert.Check(t, err == nil, "Unexpected error")
+			}
+		})
+	}
 }
 
 func TestCreateTagSuccess(t *testing.T) {