From 54d021ef8f56f90cc4f7fc604190560361dd6b0c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 5 Aug 2019 23:34:57 +0200 Subject: [PATCH 1/3] awslogs: remove unused eventBuffer update We return immediately after this, so no need to update eventBuffer: ``` 16:04:35 daemon/logger/awslogs/cloudwatchlogs.go:554:5: SA4006: this value of `eventBuffer` is never used (staticcheck) 16:04:35 eventBuffer = eventBuffer[:0] ``` Signed-off-by: Sebastiaan van Stijn --- daemon/logger/awslogs/cloudwatchlogs.go | 1 - 1 file changed, 1 deletion(-) diff --git a/daemon/logger/awslogs/cloudwatchlogs.go b/daemon/logger/awslogs/cloudwatchlogs.go index f9cf1a9840..d01deef369 100644 --- a/daemon/logger/awslogs/cloudwatchlogs.go +++ b/daemon/logger/awslogs/cloudwatchlogs.go @@ -551,7 +551,6 @@ func (l *logStream) collectBatch(created chan bool) { if !more { // Flush event buffer and release resources l.processEvent(batch, eventBuffer, eventBufferTimestamp) - eventBuffer = eventBuffer[:0] l.publishBatch(batch) batch.reset() return From ef2872132d40ba56b29f7c8a08492ebe3abb1f7c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 5 Aug 2019 23:46:43 +0200 Subject: [PATCH 2/3] awslogs: replace deprecated session.New() with session.NewSession() ``` 16:04:35 daemon/logger/awslogs/cloudwatchlogs.go:312:25: SA1019: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made. (staticcheck) 16:04:35 return ec2metadata.New(session.New()) ``` Signed-off-by: Sebastiaan van Stijn --- daemon/logger/awslogs/cloudwatchlogs.go | 19 +++++++++++++------ daemon/logger/awslogs/cloudwatchlogs_test.go | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/daemon/logger/awslogs/cloudwatchlogs.go b/daemon/logger/awslogs/cloudwatchlogs.go index d01deef369..b2a29f7f06 100644 --- a/daemon/logger/awslogs/cloudwatchlogs.go +++ b/daemon/logger/awslogs/cloudwatchlogs.go @@ -308,8 +308,12 @@ var strftimeToRegex = map[string]string{ // newRegionFinder is a variable such that the implementation // can be swapped out for unit tests. -var newRegionFinder = func() regionFinder { - return ec2metadata.New(session.New()) +var newRegionFinder = func() (regionFinder, error) { + s, err := session.NewSession() + if err != nil { + return nil, err + } + return ec2metadata.New(s), nil } // newSDKEndpoint is a variable such that the implementation @@ -333,12 +337,15 @@ func newAWSLogsClient(info logger.Info) (api, error) { } if region == nil || *region == "" { logrus.Info("Trying to get region from EC2 Metadata") - ec2MetadataClient := newRegionFinder() + ec2MetadataClient, err := newRegionFinder() + if err != nil { + logrus.WithError(err).Error("could not create EC2 metadata client") + return nil, errors.Wrap(err, "could not create EC2 metadata client") + } + r, err := ec2MetadataClient.Region() if err != nil { - logrus.WithFields(logrus.Fields{ - "error": err, - }).Error("Could not get region from EC2 metadata, environment, or log option") + logrus.WithError(err).Error("Could not get region from EC2 metadata, environment, or log option") return nil, errors.New("Cannot determine region for awslogs driver") } region = &r diff --git a/daemon/logger/awslogs/cloudwatchlogs_test.go b/daemon/logger/awslogs/cloudwatchlogs_test.go index 7ed52ee19d..5aaa5d7ec1 100644 --- a/daemon/logger/awslogs/cloudwatchlogs_test.go +++ b/daemon/logger/awslogs/cloudwatchlogs_test.go @@ -172,8 +172,8 @@ func TestNewAWSLogsClientRegionDetect(t *testing.T) { } mockMetadata := newMockMetadataClient() - newRegionFinder = func() regionFinder { - return mockMetadata + newRegionFinder = func() (regionFinder, error) { + return mockMetadata, nil } mockMetadata.regionResult <- ®ionResult{ successResult: "us-east-1", From 2e0cafb01bf7871cb66f98da6123d36ffe043823 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 5 Aug 2019 17:37:57 -0700 Subject: [PATCH 3/3] awslogs: refactor create() Get rid of too many nested if statements. Remove the redundand check for err != nil, fixing the following lint issue: > daemon/logger/awslogs/cloudwatchlogs.go:452:10: nilness: tautological condition: non-nil != nil (govet) > if err != nil { > ^ Signed-off-by: Kir Kolyshkin --- daemon/logger/awslogs/cloudwatchlogs.go | 27 ++++++++++--------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/daemon/logger/awslogs/cloudwatchlogs.go b/daemon/logger/awslogs/cloudwatchlogs.go index b2a29f7f06..aafebe6274 100644 --- a/daemon/logger/awslogs/cloudwatchlogs.go +++ b/daemon/logger/awslogs/cloudwatchlogs.go @@ -436,25 +436,20 @@ func (l *logStream) Close() error { // create creates log group and log stream for the instance of the awslogs logging driver func (l *logStream) create() error { - if err := l.createLogStream(); err != nil { - if l.logCreateGroup { - if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == resourceNotFoundCode { - if err := l.createLogGroup(); err != nil { - return errors.Wrap(err, "failed to create Cloudwatch log group") - } - err := l.createLogStream() - if err != nil { - return errors.Wrap(err, "failed to create Cloudwatch log stream") - } - return nil - } + err := l.createLogStream() + if err == nil { + return nil + } + if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == resourceNotFoundCode && l.logCreateGroup { + if err := l.createLogGroup(); err != nil { + return errors.Wrap(err, "failed to create Cloudwatch log group") } - if err != nil { - return errors.Wrap(err, "failed to create Cloudwatch log stream") + err = l.createLogStream() + if err == nil { + return nil } } - - return nil + return errors.Wrap(err, "failed to create Cloudwatch log stream") } // createLogGroup creates a log group for the instance of the awslogs logging driver