[AWSINTS-3516] refactor(go-forwarder)#1115
Conversation
| entries := make([]model.LogEntry, maxItemsPerBatch+1) | ||
| for i := range entries { | ||
| entries[i] = makeTestLogEntry() | ||
| entries[i] = model.NewLogEntry() |
There was a problem hiding this comment.
🥜 nitpick: The added value of the NewLogEntry is to put aws as the sourcecategory. I don't think it's strictly necessary for the test.
We could avoid doing the loop on entries, the line 49 is already allocating the slice with non-pointer struct, so struct are already allocated (empty).
| if includeMatch != "" { | ||
| re, includeErr := regexp.Compile(includeMatch) | ||
| if includeErr != nil { | ||
| err = errors.Join(err, fmt.Errorf("compile custom include: %w", includeErr)) | ||
| } else { | ||
| filter.includeRegex = re | ||
| } | ||
| } | ||
| if excludeMatch != "" { | ||
| re, excludeErr := regexp.Compile(excludeMatch) | ||
| if excludeErr != nil { | ||
| err = errors.Join(err, fmt.Errorf("compile custom exclude: %w", excludeErr)) | ||
| } else { | ||
| filter.excludeRegex = re | ||
| } | ||
| } |
There was a problem hiding this comment.
💬 suggestion: Can we factorize the "check empty, then compile and return error" to avoid repetition here?
Also, add blank line between "section" of the function to make it easier to read.
|
|
||
| batches := make(chan []byte) | ||
| batcher := batching.NewBatcher() | ||
| g.Go(func() error { |
There was a problem hiding this comment.
💬 suggestion: Usually, the name for errgroup is eg or errgroup. Long-shot feedback is to avoid one letter variable name (i is tolerated).
| } | ||
| }() | ||
|
|
||
| var d events.CloudwatchLogsData |
There was a problem hiding this comment.
Same comment here regarding one letter variable name
2 participants
What does this PR do?
Improve performance, refactor the pipeline and log entry structures.
Motivation
Testing Guidelines
Additional Notes
Types of changes
Check all that apply