[AWSINTS-3563] refactor(go-forwarder)#1114
Conversation
| batches := make(chan []byte) | ||
|
|
||
| batcher := processing.NewBatcher[T]() | ||
| forwarder := forwarding.NewForwarder(cfg, &http.Client{ |
There was a problem hiding this comment.
I'm not sure why you created the client within forwading and didn't pass it here directly ? is it used somewhere else ?
There was a problem hiding this comment.
I passed it to the Forwarder struct to make testing easier. The forwarding.Client is an http.Client which is initialized when the lambda cold starts and is reused across invocations.
| var jsonMessage map[string]any | ||
| if err := json.Unmarshal([]byte(message), &jsonMessage); err != nil { | ||
| return nil, service, message | ||
| return |
There was a problem hiding this comment.
please let's not use this syntax it's confusing. What's your motivation here ?
There was a problem hiding this comment.
I wasn't sure if it was idiomatic to use returned parameters here. I agree that the initial newMessage = message makes it difficult to read and I'll revert that.
| GetObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) | ||
| } | ||
|
|
||
| func getS3APIClient(ctx context.Context, useFIPS bool) (S3APIClient, error) { |
There was a problem hiding this comment.
if you're calling this once already, why would you need a once here ? I saw a single call for createS3APIClient
There was a problem hiding this comment.
Doing so makes us lazy load the AWS SDK client and reuse it across lambda invocations.
| func newS3EntryBase(record events.S3EventRecord, cfg *config.Config, lambdaOrigin model.LambdaOrigin) s3EntryBase { | ||
| bucket := record.S3.Bucket.Name | ||
| key := record.S3.Object.URLDecodedKey | ||
| source := cmp.Or(cfg.Source, getS3Source(key)) |
There was a problem hiding this comment.
are you sure you want to use cmp.Or here ?
Or returns the first of its arguments that is not equal to the zero value. If no argument is non-zero, it returns the zero value.
There was a problem hiding this comment.
The overriden source (configured through the DD_SOURCE env var) has the higher priority, so whenever it is configured it will be used. Otherwise we fallback to the key extraction.
2 participants
What does this PR do?
Refactors names, adds lazy loading and recent go features
Motivation
Testing Guidelines
Additional Notes
LogEntrytype.S3LogEntryis a now a plainLogEntry.Metadata→LambdaOriginCloudwatchLogsContext→CloudwatchOriginS3Context→S3OriginHasPrefix+ manual slicing withstrings.CutPrefix.maxTokenSizefrom 512KB to ~1MB.context.Background()tot.Context()across all test file,defer server.Close()/defer cancel()tot.Cleanup()and added missingt.Parallel().Types of changes
Check all that apply