Add logs agent integ test #433

varunch77 · 2024-11-19T14:54:18Z

Description of the issue

We are currently missing tests that ensure that the logs agent is properly sending an entity when sending application logs to CloudWatch Logs.

Description of changes

TestWriteLogsWithEntityInfo() writes logs to CloudWatch and validates that the logs are associated with entities and that all values are translated properly. The three test cases are IAMRole, ServiceInConfig (which uses the added config_log_service_name.json configuration), and EC2Tags. The changes also include a suite of helper functions like ValidateEntity() to validate the logs and check that the expected entity information is present after querying the published logs.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

I ran all tests in the publish_logs_test.go file on a newly created EC2 instance with the most recent build of the cloudwatch agent and the end-to-end testing IAM role attached. I also verified that TestWriteLogsWithEntityInfo passed for every running order combination of the IAM, EC2, and Service test cases.

Test run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/12283029637

lisguo · 2024-11-21T20:38:13Z

test/cloudwatchlogs/publish_logs_test.go

 	configPathAutoRemoval         = "resources/config_auto_removal.json"
 	standardLogGroupClass         = "STANDARD"
 	infrequentAccessLogGroupClass = "INFREQUENT_ACCESS"
+	cwlPerfEndpoint               = "https://logs.us-west-2.amazonaws.com"


This relies on the fact that we are running the test in us-west-2...

I know we only run our tests in us-west-2 but can we detect the region on the host and then populate the endpoint accordingly so that in the future we can support other regions?

I think we may already be constrained by the region being hardcoded elsewhere: https://github.com/aws/amazon-cloudwatch-agent-test/blob/main/util/awsservice/constant.go#L58C1-L60C78. We would need to make edits there but I'm a little wary of what the blast radius of that would look like for our testing

We don't need to hardcode the endpoint here. The code I wrote originally had to use a custom endpoint because it was a beta endpoint but this is the public us-west-2 endpoint which the client will automatically resolve

lisguo · 2024-11-21T20:38:59Z

test/cloudwatchlogs/publish_logs_test.go

@@ -70,6 +88,9 @@ var (
 			logGroupClass: types.LogGroupClassInfrequentAccess,
 		},
 	}
+	rnf       *types.ResourceNotFoundException


nit: just spell out the entire name

Suggested change

rnf *types.ResourceNotFoundException

resourceNotFoundException *types.ResourceNotFoundException

addressed in commit 84cc3d3

lisguo · 2024-11-21T20:39:46Z

test/cloudwatchlogs/publish_logs_test.go

+	)
+	if err != nil {
+		fmt.Println("There was an error trying to load default config: ", err)
+		return


Does this fail the test?

It didn't, but I changed the behavior so that it will fail now if the default config fails in commit 84cc3d3

lisguo · 2024-11-21T21:01:51Z

test/cloudwatchlogs/publish_logs_test.go

@@ -119,9 +161,9 @@ func TestWriteLogsToCloudWatch(t *testing.T) {

 			// ensure that there is enough time from the "start" time and the first log line,
 			// so we don't miss it in the GetLogEvents call
-			time.Sleep(sleepForFlush)
+			time.Sleep(sleepForExtendedFlush)


Why not just increase sleepForFlush? Do we use sleepForFlush anywhere else in the code?

yeah we use sleepForFlush in other places too. For the other test cases, waiting for 180 seconds instead of 20 seconds would be overkill and sometimes results in a timeout as it can cause all the test cases in the file take over an hour to run. This is because we call sleepForFlush before and after the agent runs and some of the test cases are set to loop multiple times so this extra 160 seconds adds up quickly. We only want the tests to wait for 180 seconds in two places, which is why I just created another variable.

lisguo · 2024-11-21T21:05:49Z

test/cloudwatchlogs/publish_logs_test.go

+
+			ValidateEntity(t, instanceId, instanceId, &end, testCase.expectedEntity)
+
+			f.Close()


feel like we can f.Close() after we are done writing on line 285

deferred both of these calls

lisguo · 2024-11-21T21:16:46Z

test/cloudwatchlogs/publish_logs_test.go

+	log.Printf("Validating entity for log group: %s, stream: %s", logGroup, logStream)
+
+	logGroupInfo, err := getLogGroup()
+	for _, lg := range logGroupInfo {


Is there no way to get a specific log group when you provide the name?

Seems expensive to get all log groups in an account. We have a ton in our test account..

I don't think there's a way to just search for one log group (why not is beyond me..) but I updated this so it passes in the log group as a prefix to search by (commit 006a057)

lisguo · 2024-11-21T21:17:15Z

test/cloudwatchlogs/publish_logs_test.go

+		}
+	}
+	assert.NoError(t, err)
+	begin := end.Add(-sleepForExtendedFlush * 4)


Instead of trying to calculate the begin time, can we just pass in the begin time?

Updated this for ValidateEntity() (commit 7950725)

lisguo · 2024-11-21T21:18:07Z

test/cloudwatchlogs/publish_logs_test.go

+func getLogQueryId(logGroup string, since, until *time.Time) (*string, error) {
+	var queryId *string
+	params := &cloudwatchlogs.StartQueryInput{
+		QueryString:  aws.String("fields @message, @entity.KeyAttributes.Type, @entity.KeyAttributes.Name, @entity.KeyAttributes.Environment, @entity.Attributes.PlatformType, @entity.Attributes.EC2.InstanceId"),


nit: make this a const

addressed in commit 84cc3d3

lisguo · 2024-11-21T21:20:57Z

test/cloudwatchlogs/publish_logs_test.go

+	if !assert.NotZero(t, len(result)) {
+		return
+	}
+	requiredEntityFields := map[string]bool{


Not sure I understand why we need this...when we pass in the query:

fields @message, @entity.KeyAttributes.Type, @entity.KeyAttributes.Name, @entity.KeyAttributes.Environment, @entity.Attributes.PlatformType, @entity.Attributes.EC2.InstanceId

Doesn't the fact that there are results mean that the logs all have the required fields?

Line 510 isn't checking the content of the logs but instead checking to see that our query actually returned logs at all. If our query came back empty handed, we want to fail immediately instead of attempting to validate it and check that its fields match our expectations.

varunch77 · 2024-11-22T21:17:42Z

New run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/11978587627

zhihonl · 2024-11-26T15:31:38Z

test/cloudwatchlogs/publish_logs_test.go

 	configPathAutoRemoval         = "resources/config_auto_removal.json"
 	standardLogGroupClass         = "STANDARD"
 	infrequentAccessLogGroupClass = "INFREQUENT_ACCESS"
+	cwlPerfEndpoint               = "https://logs.us-west-2.amazonaws.com"
+	pdxRegionalCode               = "us-west-2"


region hardcoding is not needed since entity feature is GA

zhihonl · 2024-11-26T15:32:27Z

test/cloudwatchlogs/publish_logs_test.go

+		log.Fatalf("Failed to load default config: %v", err)
+	}
+
+	cwlClient = cloudwatchlogs.NewFromConfig(awsCfg, func(o *cloudwatchlogs.Options) {


we don't need to create a new client. we have globally available client that already uses us-west-2 as the region" https://github.com/aws/amazon-cloudwatch-agent-test/blob/main/util/awsservice/constant.go#L74

zhihonl · 2024-11-26T15:32:37Z

test/cloudwatchlogs/publish_logs_test.go

+	cwlClient = cloudwatchlogs.NewFromConfig(awsCfg, func(o *cloudwatchlogs.Options) {
+		o.BaseEndpoint = aws.String(cwlPerfEndpoint)
+	})
+	ec2Client = ec2.NewFromConfig(awsCfg)


same as above

zhihonl · 2024-11-26T15:35:41Z

test/cloudwatchlogs/publish_logs_test.go

+func ValidateEntity(t *testing.T, logGroup, logStream string, begin, end *time.Time, expectedEntity expectedEntity) {
+	log.Printf("Validating entity for log group: %s, stream: %s", logGroup, logStream)
+
+	logGroupInfo, err := getLogGroup(logGroup)


You can just use IsLogGroupExists from our common package: https://github.com/aws/amazon-cloudwatch-agent-test/blob/main/util/awsservice/cloudwatchlogs.go#L137

The helper functions that I wrote was because we had to create a new client in us-east-1 for beta purpose but they are no longer needed

zhihonl · 2024-11-26T15:36:25Z

test/cloudwatchlogs/publish_logs_test.go

+	queryId, err := getLogQueryId(logGroup, begin, end)
+	assert.NoError(t, err)
+	log.Printf("queryId is " + *queryId)
+	result, err := getQueryResult(queryId)


getLogQueryId and getQueryResult are not needed anymore since we can just use default client now. There's a helper function here: https://github.com/aws/amazon-cloudwatch-agent-test/blob/main/util/awsservice/cloudwatchlogs.go#L202

zhihonl · 2024-11-26T15:38:13Z

test/cloudwatchlogs/publish_logs_test.go

@@ -332,3 +497,154 @@ func checkData(t *testing.T, start time.Time, lineCount int) {
 	)
 	assert.NoError(t, err)
 }
+
+func ValidateEntity(t *testing.T, logGroup, logStream string, begin, end *time.Time, expectedEntity expectedEntity) {


nonblocking but recommended: This function has a lot of overlaps with this function: https://github.com/aws/amazon-cloudwatch-agent-test/blob/main/test/entity/entity_test.go#L241

Maybe good idea to move ValidateEntity to a common package and just use for any entity validation tests in the future.

…test

varunch77 · 2024-12-05T13:00:38Z

Addressed comments above, including merging ValidateEntity with ValidateLogEntity and moving the resulting function to the util package so it can be used in multiple places for future entity validation tests.

New run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/12283029637

varunch77 added 10 commits November 18, 2024 19:53

update config files

21aa051

Add integ test for Logs agent

734f8e0

Have the agent sleep after starting it

5ed1a93

update flush time to 90s

96522cd

Update flush time to 120 seconds

b98c612

Add delay to ensure tag deletion propagates

bdd40b9

Change order of tests

43d2ca6

Change order of tests again

9f28f03

Enable instance metadata tags

e1bb9ce

Clean up unused code

fd59713

varunch77 requested a review from a team as a code owner November 19, 2024 14:54

varunch77 and others added 2 commits November 19, 2024 18:59

Differentiate sleep times

6e00757

Merge branch 'main' into add-logs-agent-integ-test

bb229f7

lisguo reviewed Nov 21, 2024

View reviewed changes

varunch77 added 7 commits November 22, 2024 16:28

Address nits and make minor changes

84cc3d3

UGet region from EC2 metadata instead of hardcoding

7a3b428

Revert EC2 metadata change

e37d84a

Revert EC2 metadata change

79744f1

Address misc comments

7950725

update code to search for log group directly

37cb8a5

Revert to search by prefix

006a057

zhihonl reviewed Nov 26, 2024

View reviewed changes

varunch77 added 2 commits December 4, 2024 15:55

address comments

eb95406

remove unused packageS

df7abce

varunch77 added 7 commits December 4, 2024 16:39

use clients from the awsservice package

8dcc2d4

Merge remote-tracking branch 'origin/main' into add-logs-agent-integ-…

9f5722b

…test

Make ValidateEntity a common package

6f1e4cf

modify ValidateLogs to be a function in a common package

5bd4a65

Merge remote-tracking branch 'origin/main' into add-logs-agent-integ-…

d116feb

…test

Increase flush time

6c272ee

run linter

58f0a88

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logs agent integ test #433

Add logs agent integ test #433

varunch77 commented Nov 19, 2024 •

edited

Loading

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

zhihonl Nov 26, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

lisguo Nov 21, 2024

varunch77 Nov 22, 2024

varunch77 commented Nov 22, 2024

zhihonl Nov 26, 2024

zhihonl Nov 26, 2024

zhihonl Nov 26, 2024

zhihonl Nov 26, 2024

zhihonl Nov 26, 2024

zhihonl Nov 26, 2024

varunch77 commented Dec 5, 2024 •

edited

Loading

	rnf *types.ResourceNotFoundException
	resourceNotFoundException *types.ResourceNotFoundException


		ValidateEntity(t, instanceId, instanceId, &end, testCase.expectedEntity)

		f.Close()

Add logs agent integ test #433

Are you sure you want to change the base?

Add logs agent integ test #433

Conversation

varunch77 commented Nov 19, 2024 • edited Loading

Description of the issue

Description of changes

License

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunch77 commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunch77 commented Dec 5, 2024 • edited Loading

varunch77 commented Nov 19, 2024 •

edited

Loading

varunch77 commented Dec 5, 2024 •

edited

Loading