system-tests icon indicating copy to clipboard operation
system-tests copied to clipboard

Change AWS messaging propagation & DSM tests to use actual AWS

Open wconti27 opened this issue 1 year ago • 1 comments
trafficstars

Motivation

This PR initially began to simply enable a few more Java AWS SNS propagation tests, now that dd-trace-java has the functionality to propagate APM and DSM context through SNS topics. In enabling these tests, it was discovered that Localstack, the 3rd party Docker image used to mock AWS services, does not provide the functionality needed to properly test these new features.

It was decided to move all tests to AWS, which should boost our confidence in these propagation tests.

Changes

General Changes

  • Remove Localstack container image
  • Change AWS clients to reference our Datadog Sandbox Eng account
  • Add AWS auth tokens via environment variables provided through Github secrets
  • A distinct hash UNIQUE_ID created that will be appended to all AWS resource names (queue names, topic names, etc) to avoid conflicting resources between different system-tests CI runs.

Test Specific Changes

Cross Propagation Messaging Integration Tests:

  • Add distinct id as AWS SQS Queue, SNS Topic, and Kinesis stream names. Prevents test resources conflicting.
  • Add functions to cleanup test resources used in AWS after setup functions have run
  • Add distinct id to the message being sent, and have consumer services stop consuming once exact message is found

DSM Test Updates:

  • Add distinct id to AWS queue / topic names.
  • Add distinct id to message sent through queues
  • Since queue names are now dynamic, we need to calculate the DSM hash for each test case. The DSM hashing function from dd-trace-py has been added to test utils.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

:rocket: Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • [ ] If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • [ ] No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • [ ] CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • [ ] A docker base image is modified?
    • [ ] the relevant build-XXX-image label is present
  • [ ] A scenario is added (or removed)?

wconti27 avatar Jul 11 '24 18:07 wconti27

I set it as draft, you can set it back to normal when you need a review

cbeauchesne avatar Jul 24 '24 15:07 cbeauchesne

Thanks William. Awesome PR!

One question - is it at all possible to spilt the testing into one part working with "localstack" and the second "extended" part to work with AWS ?

I'm asking mostly from 3 perspectives:

  1. avoiding engineers locally to setup aws credentials to run at least some basic tests
  2. avoiding flakiness of actuall AWS
  3. and most importantly - we might actually have to put some of those tests in GitLab to avoid exposing secrets to our AWS accounts on public CIs

pawelchcki avatar Aug 13 '24 12:08 pawelchcki

Ok, so far LGTM.

BUT :)

Every time we rely on an external service in testing, we are prone to this service healthiness. I think we can give it a try, but at the first sign of flakiness, I think it's better to move those tests in a dedicated scenario, to not change the signal quality of CROSSED_INTEGRATION and INTEGRATION.

Does it make sense for you ?

Sounds good to me!

wconti27 avatar Sep 13 '24 13:09 wconti27