-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-add AWS CloudWatch source #131
Conversation
d514484
to
39a8b11
Compare
// It ensures that the required LogGroupName field is provided and not empty. | ||
func (c *AwsCloudWatchSourceConfig) Validate() error { | ||
if c.LogGroupName == "" { | ||
return fmt.Errorf("log group is required and cannot be empty") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the hcl field name log_group_name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would be more appropriate to use the HCL field name. Updated the statement
// AWS CloudWatch Logs client | ||
client *cloudwatchlogs.Client | ||
// List of errors encountered during collection | ||
errorList []error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to be using this, we instantiate it in Init func but never append to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used the errorList
property to collect all the errors during the collection.
// getClient initializes and returns an AWS CloudWatch Logs client | ||
// It uses the provided region or falls back to the default region | ||
func (s *AwsCloudWatchSource) getClient(ctx context.Context) (*cloudwatchlogs.Client, error) { | ||
tempRegion := defaultCloudwatchRegion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tempRegion
? I'd just call it region
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use region
instead
isFirst := true | ||
|
||
for _, state := range s.LogStreams { | ||
if isFirst { | ||
earliestTime = state.GetStartTime() | ||
isFirst = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a bool, you can check the index of the iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
isFirst := true | ||
|
||
for _, state := range s.LogStreams { | ||
if isFirst { | ||
latestTime = state.GetEndTime() | ||
isFirst = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of bool, could use index of iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
|
||
// GetFromTimeForStream returns the start time for a specific log stream. | ||
// If the stream doesn't exist in the state, returns zero time. | ||
func (s *CloudWatchCollectionState) GetFromTimeForStream(logStreamName string) time.Time { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're pulling StartTime, GetStartTimeForStream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
|
||
// GetToTimeForStream returns the end time for a specific log stream. | ||
// If the stream doesn't exist in the state, returns zero time. | ||
func (s *CloudWatchCollectionState) GetToTimeForStream(logStreamName string) time.Time { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're pulling EndTime, GetEndTimeForStream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Example query results
Results