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 entity integration tests for statsd and collectd metrics #463

Open
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

varunch77
Copy link
Member

@varunch77 varunch77 commented Feb 19, 2025

Description of the issue

We are currently missing integration tests that ensure that we are sending a service entity when publishing statsd/collectd metrics.

Description of changes

Added to the existing statsd/collectd tests to make a ListEntitiesForMetric call to validate that an entity is being sent.

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

Integration test run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/13688151654

@varunch77 varunch77 requested a review from a team as a code owner February 19, 2025 19:01
varunch77 and others added 11 commits February 19, 2025 14:49
* Copy amp destination test to amp histogram test

* Only generate histogram test

* Refactoring all AMP tests into one. Need to rename later

* add more metric types

* Rough test for all supported metrics

* Code cleanup

* Bump linter version

Previous version crashes

* Move new test logic back to original folder

* more code cleanup

* Surpress s3 cp progress

* surpress git clone progress

* otlp_pusher.sh can be executed

* few debugging improvements

* Minor error handling improvement

* restore test case generator

* Move amp test up to main directory

* Address code review comments

* Remove unused label for prometheus
@TravisStark
Copy link
Contributor

Could you add a sample snippet/screenshot showing specifically how this test output is different after this change?

@varunch77
Copy link
Member Author

Could you add a sample snippet/screenshot showing specifically how this test output is different after this change?

The actual test output is unchanged. This PR uses existing StatsD/CollectD tests and adds an extra validation step to check that an entity is being sent with these metrics.

@zhihonl
Copy link
Contributor

zhihonl commented Mar 5, 2025

Can we modify the test code so we can explicitly see if entity tests passed? Maybe as a separate row of test case

For example, looking at StatsD and CollectD section of the test output: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/13681447031/job/38254861506#step:7:3088

It's unclear whether the entity is tested or not because this PR is re-using existing test logic then adding on the entity check into that.


switch computeType {
case "EC2":
requestBody = []byte(fmt.Sprintf(`{
Copy link
Contributor

@zhihonl zhihonl Mar 5, 2025

Choose a reason for hiding this comment

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

Can we make a helper function to create these requests dynamically? Something that inputs dynamic amount of dimensions with their values? It would make the code here less clunky

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.

6 participants