Skip to content

Conversation

@pxaws
Copy link
Member

@pxaws pxaws commented Feb 14, 2025

Issue description:

Description of changes:

Shorten the remote deployment name so that the relevant pod name follow the regular regex pattern. See details here: aws/amazon-cloudwatch-agent#1553

Rollback procedure:

<Can we safely revert this commit if needed? If not, detail what must be done to safely revert and why it is needed.>

Ensure you've run the following tests on your changes and include the link below:

To do so, create a test.yml file with name: Test and workflow description to test your changes, then remove the file for your PR. Link your test run in your PR description. This process is a short term solution while we work on creating a staging environment for testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@pxaws pxaws marked this pull request as draft February 14, 2025 01:28
@pxaws pxaws force-pushed the rename-remote-service branch from c415256 to 70bc5a1 Compare February 14, 2025 01:58
@pxaws pxaws marked this pull request as ready for review February 14, 2025 02:01
@pxaws pxaws force-pushed the rename-remote-service branch from 431c04c to 376db95 Compare February 14, 2025 17:39
@pxaws pxaws force-pushed the rename-remote-service branch from 376db95 to 0e97069 Compare February 14, 2025 17:52
Copy link
Contributor

@majanjua-amzn majanjua-amzn left a comment

Choose a reason for hiding this comment

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

I would prefer if we can still identify that this value refers to the deployment.

I would also prefer that we validate the resulting behaviour, not tailor the tests to get the telemetry we want. This feels backwards, instead of updating the setup we should be testing that having a long name results in the correct service name. We are simply avoiding the issue with this PR and we lose visibility.

@pxaws
Copy link
Member Author

pxaws commented Feb 14, 2025

I would prefer if we can still identify that this value refers to the deployment.

I would also prefer that we validate the resulting behaviour, not tailor the tests to get the telemetry we want. This feels backwards, instead of updating the setup we should be testing that having a long name results in the correct service name. We are simply avoiding the issue with this PR and we lose visibility.

We have python, dotnet, node tests still use the shorter deployment. In this case, the Remote Service will take the value of the deployment. So they are still testing the old behavior as before. For the java test, we choose to keep the long deployment name and set the service name to be the same as deployment name (to test the edge case where we will use the service name if pod name doesn't follow normal regex pattern).

@pxaws pxaws merged commit 3d1dc43 into main Feb 14, 2025
1 check passed
@pxaws pxaws deleted the rename-remote-service branch February 14, 2025 22:13
pxaws added a commit that referenced this pull request Feb 14, 2025
pxaws added a commit that referenced this pull request Feb 15, 2025
This reverts commit 3d1dc43.

*Issue description:*

*Description of changes:*

*Rollback procedure:*

<Can we safely revert this commit if needed? If not, detail what must be
done to safely revert and why it is needed.>

*Ensure you've run the following tests on your changes and include the
link below:*

To do so, create a `test.yml` file with `name: Test` and workflow
description to test your changes, then remove the file for your PR. Link
your test run in your PR description. This process is a short term
solution while we work on creating a staging environment for testing.

NOTE: TESTS RUNNING ON A SINGLE EKS CLUSTER CANNOT BE RUN IN PARALLEL.
See the
[needs](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds)
keyword to run tests in succession.
- Run Java EKS on `e2e-playground` in us-east-1 and eu-central-2
- Run Python EKS on `e2e-playground` in us-east-1 and eu-central-2
- Run metric limiter on EKS cluster `e2e-playground` in us-east-1 and
eu-central-2
- Run EC2 tests in all regions
- Run K8s on a separate K8s cluster (check IAD test account for master
node endpoints; these will change as we create and destroy clusters for
OS patching)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
pxaws added a commit that referenced this pull request Feb 15, 2025
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.

2 participants