Skip to content

Commit

Permalink
[internal/aws/cwlogs] Enabling gocritic / errcheck (open-telemetry#12328
Browse files Browse the repository at this point in the history
)

Enabling gocritic / errcheck for internal/aws/cwlogs

Signed-off-by: John D <[email protected]>
  • Loading branch information
boostchicken committed Jul 12, 2022
1 parent 4f30925 commit 7b2b26b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 33 deletions.
21 changes: 10 additions & 11 deletions internal/aws/cwlogs/cwlog_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package cwlogs // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs"

import (
Expand Down Expand Up @@ -47,7 +46,7 @@ type Client struct {
logger *zap.Logger
}

//Create a log client based on the actual cloudwatch logs client.
// Create a log client based on the actual cloudwatch logs client.
func newCloudWatchLogClient(svc cloudwatchlogsiface.CloudWatchLogsAPI, logger *zap.Logger) *Client {
logClient := &Client{svc: svc,
logger: logger}
Expand All @@ -62,8 +61,8 @@ func NewClient(logger *zap.Logger, awsConfig *aws.Config, buildInfo component.Bu
return newCloudWatchLogClient(client, logger)
}

//PutLogEvents mainly handles different possible error could be returned from server side, and retries them
//if necessary.
// PutLogEvents mainly handles different possible error could be returned from server side, and retries them
// if necessary.
func (client *Client) PutLogEvents(input *cloudwatchlogs.PutLogEventsInput, retryCnt int) (*string, error) {
var response *cloudwatchlogs.PutLogEventsOutput
var err error
Expand All @@ -82,18 +81,18 @@ func (client *Client) PutLogEvents(input *cloudwatchlogs.PutLogEventsInput, retr
case *cloudwatchlogs.InvalidParameterException:
client.logger.Error("cwlog_client: Error occurs in PutLogEvents, will not retry the request", zap.Error(e), zap.String("LogGroupName", *input.LogGroupName), zap.String("LogStreamName", *input.LogStreamName))
return token, err
case *cloudwatchlogs.InvalidSequenceTokenException: //Resend log events with new sequence token when InvalidSequenceTokenException happens
case *cloudwatchlogs.InvalidSequenceTokenException: // Resend log events with new sequence token when InvalidSequenceTokenException happens
client.logger.Warn("cwlog_client: Error occurs in PutLogEvents, will search the next token and retry the request", zap.Error(e))
token = e.ExpectedSequenceToken
continue
case *cloudwatchlogs.DataAlreadyAcceptedException: //Skip batch if DataAlreadyAcceptedException happens
case *cloudwatchlogs.DataAlreadyAcceptedException: // Skip batch if DataAlreadyAcceptedException happens
client.logger.Warn("cwlog_client: Error occurs in PutLogEvents, drop this request and continue to the next request", zap.Error(e))
token = e.ExpectedSequenceToken
return token, err
case *cloudwatchlogs.OperationAbortedException: //Retry request if OperationAbortedException happens
case *cloudwatchlogs.OperationAbortedException: // Retry request if OperationAbortedException happens
client.logger.Warn("cwlog_client: Error occurs in PutLogEvents, will retry the request", zap.Error(e))
return token, err
case *cloudwatchlogs.ServiceUnavailableException: //Retry request if ServiceUnavailableException happens
case *cloudwatchlogs.ServiceUnavailableException: // Retry request if ServiceUnavailableException happens
client.logger.Warn("cwlog_client: Error occurs in PutLogEvents, will retry the request", zap.Error(e))
return token, err
case *cloudwatchlogs.ResourceNotFoundException:
Expand Down Expand Up @@ -144,9 +143,9 @@ func (client *Client) PutLogEvents(input *cloudwatchlogs.PutLogEventsInput, retr
return token, err
}

//Prepare the readiness for the log group and log stream.
// Prepare the readiness for the log group and log stream.
func (client *Client) CreateStream(logGroup, streamName *string) (token string, e error) {
//CreateLogStream / CreateLogGroup
// CreateLogStream / CreateLogGroup
_, err := client.svc.CreateLogStream(&cloudwatchlogs.CreateLogStreamInput{
LogGroupName: logGroup,
LogStreamName: streamName,
Expand Down Expand Up @@ -176,7 +175,7 @@ func (client *Client) CreateStream(logGroup, streamName *string) (token string,
return token, err
}

//After a log stream is created the token is always empty.
// After a log stream is created the token is always empty.
return "", nil
}

Expand Down
3 changes: 1 addition & 2 deletions internal/aws/cwlogs/cwlog_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package cwlogs

import (
Expand Down Expand Up @@ -186,7 +185,7 @@ func TestPutLogEvents_InvalidSequenceTokenException(t *testing.T) {
NextSequenceToken: &expectedNextSequenceToken}
awsErr := &cloudwatchlogs.InvalidSequenceTokenException{ExpectedSequenceToken: &expectedNextSequenceToken}

//the test framework does not support return different result sequentially for the same method call.
// the test framework does not support return different result sequentially for the same method call.
svc.On("PutLogEvents", putLogEventsInput).Return(putLogEventsOutput, awsErr).Once()
svc.On("PutLogEvents", putLogEventsInput).Return(putLogEventsOutput, nil).Once()

Expand Down
23 changes: 11 additions & 12 deletions internal/aws/cwlogs/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package cwlogs // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs"

import (
Expand All @@ -29,7 +28,7 @@ import (
const (
// http:https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/cloudwatch_limits_cwl.html
// In truncation logic, it assuming this constant value is larger than perEventHeaderBytes + len(truncatedSuffix)
defaultMaxEventPayloadBytes = 1024 * 256 //256KB
defaultMaxEventPayloadBytes = 1024 * 256 // 256KB
// http:https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_PutLogEvents.html
maxRequestEventCount = 10000
perEventHeaderBytes = 26
Expand All @@ -39,8 +38,8 @@ const (

truncatedSuffix = "[Truncated...]"

eventTimestampLimitInPast = 14 * 24 * time.Hour //None of the log events in the batch can be older than 14 days
evenTimestampLimitInFuture = -2 * time.Hour //None of the log events in the batch can be more than 2 hours in the future.
eventTimestampLimitInPast = 14 * 24 * time.Hour // None of the log events in the batch can be older than 14 days
evenTimestampLimitInFuture = -2 * time.Hour // None of the log events in the batch can be more than 2 hours in the future.
)

var (
Expand Down Expand Up @@ -82,11 +81,11 @@ func (logEvent *Event) Validate(logger *zap.Logger) error {
return errors.New("empty log event message")
}

//http:https://docs.aws.amazon.com/goto/SdkForGoV1/logs-2014-03-28/PutLogEvents
//* None of the log events in the batch can be more than 2 hours in the
//future.
//* None of the log events in the batch can be older than 14 days or the
//retention period of the log group.
// http:https://docs.aws.amazon.com/goto/SdkForGoV1/logs-2014-03-28/PutLogEvents
// * None of the log events in the batch can be more than 2 hours in the
// future.
// * None of the log events in the batch can be older than 14 days or the
// retention period of the log group.
currentTime := time.Now().UTC()
utcTime := time.Unix(0, *logEvent.InputLogEvent.Timestamp*int64(time.Millisecond)).UTC()
duration := currentTime.Sub(utcTime)
Expand All @@ -107,11 +106,11 @@ func (logEvent *Event) eventPayloadBytes() int {
// eventBatch struct to present a log event batch
type eventBatch struct {
putLogEventsInput *cloudwatchlogs.PutLogEventsInput
//the total bytes already in this log event batch
// the total bytes already in this log event batch
byteTotal int
//min timestamp recorded in this log event batch (ms)
// min timestamp recorded in this log event batch (ms)
minTimestampMs int64
//max timestamp recorded in this log event batch (ms)
// max timestamp recorded in this log event batch (ms)
maxTimestampMs int64
}

Expand Down
25 changes: 17 additions & 8 deletions internal/aws/cwlogs/pusher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:errcheck,gocritic
package cwlogs

import (
Expand Down Expand Up @@ -50,10 +49,17 @@ func TestConcurrentPushAndFlush(t *testing.T) {
for i := 0; i < concurrency; i++ {
go func(ii int) {
for j := 0; j < 10; j++ {
emfPusher.AddLogEntry(NewEvent(current, fmt.Sprintf("batch-%d-%d", ii, j)))
err := emfPusher.AddLogEntry(NewEvent(current, fmt.Sprintf("batch-%d-%d", ii, j)))
if err != nil {
t.Errorf("Error adding log entry: %v", err)
}
}
time.Sleep(1000 * time.Millisecond)
emfPusher.ForceFlush()
err := emfPusher.ForceFlush()
if err != nil {
t.Errorf("Error flushing: %v", err)

}
wg.Done()
}(i)
}
Expand Down Expand Up @@ -122,21 +128,21 @@ func TestLogEventBatch_timestampWithin24Hours(t *testing.T) {
minTimestampMs: min.UnixNano() / 1e6,
}

//less than the min
// less than the min
target := min.Add(-1 * time.Hour)
assert.True(t, logEventBatch.isActive(aws.Int64(target.UnixNano()/1e6)))

target = target.Add(-1 * time.Millisecond)
assert.False(t, logEventBatch.isActive(aws.Int64(target.UnixNano()/1e6)))

//more than the max
// more than the max
target = max.Add(1 * time.Hour)
assert.True(t, logEventBatch.isActive(aws.Int64(target.UnixNano()/1e6)))

target = target.Add(1 * time.Millisecond)
assert.False(t, logEventBatch.isActive(aws.Int64(target.UnixNano()/1e6)))

//in between min and max
// in between min and max
target = min.Add(2 * time.Hour)
assert.True(t, logEventBatch.isActive(aws.Int64(target.UnixNano()/1e6)))
}
Expand Down Expand Up @@ -208,7 +214,7 @@ func TestPusher_addLogEventBatch(t *testing.T) {
assert.Equal(t, cap, len(p.logEventBatch.putLogEventsInput.LogEvents))

assert.NotNil(t, p.addLogEvent(logEvent))
//the actual log event add operation happens after the func newLogEventBatchIfNeeded
// the actual log event add operation happens after the func newLogEventBatchIfNeeded
assert.Equal(t, 1, len(p.logEventBatch.putLogEventsInput.LogEvents))

p.logEventBatch.byteTotal = maxRequestPayloadBytes - logEvent.eventPayloadBytes() + 1
Expand Down Expand Up @@ -238,7 +244,10 @@ func TestAddLogEventWithValidation(t *testing.T) {
logEvent := NewEvent(timestampMs, largeEventContent)
expectedTruncatedContent := (*logEvent.InputLogEvent.Message)[0:(defaultMaxEventPayloadBytes-perEventHeaderBytes-len(truncatedSuffix))] + truncatedSuffix

p.AddLogEntry(logEvent)
err := p.AddLogEntry(logEvent)
if err != nil {
t.Errorf("Error adding log entry: %v", err)
}
assert.Equal(t, expectedTruncatedContent, *logEvent.InputLogEvent.Message)

logEvent = NewEvent(timestampMs, "")
Expand Down

0 comments on commit 7b2b26b

Please sign in to comment.