Skip to content
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

Add EMF concurrency test to check for log corruption. #351

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

jefchien
Copy link
Contributor

Description of the issue

There was an issue with the TCP log receiver corrupting logs on high concurrency. This was fixed as part of amazon-contributing/opentelemetry-collector-contrib#72, but was not pulled into the agent until aws/amazon-cloudwatch-agent#857. This was undetected because it was verified using manual testing.

Description of changes

Add an EMF concurrency test that creates multiple TCP connections to the agent and writes to them concurrently. Runs a log query to determine if any of the logs were corrupted.

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

Failure from v1.300028.1 (aws/amazon-cloudwatch-agent@6b25891) run
https://github.com/aws/amazon-cloudwatch-agent/actions/runs/6304504010/job/17116940882

Success from latest (aws/amazon-cloudwatch-agent@2a3fdf9) run
https://github.com/aws/amazon-cloudwatch-agent/actions/runs/6305043334/job/17117674411

@jefchien jefchien requested a review from a team as a code owner September 25, 2023 21:57
sethAmazon
sethAmazon previously approved these changes Sep 26, 2023
}
}

func (e *emitter) createEmfLog(metadata *Metadata) []byte {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to done now, but I can see this code to send EMF metrics being useful outside this test case.
Might want to move it to a UTIL at some point. I saw you mentioned there was no EMF package in go to do this... yikes.

movence
movence previously approved these changes Sep 26, 2023
@jefchien jefchien dismissed stale reviews from movence and sethAmazon via 7a1723d September 26, 2023 19:31
@jefchien jefchien requested a review from sethAmazon September 26, 2023 20:18
@jefchien jefchien merged commit bca3359 into aws:main Sep 26, 2023
2 checks passed
@jefchien jefchien deleted the concurrent-emf branch September 26, 2023 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants