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

[SYNPY-1305] Collect trace data from integration tests #1009

Merged
merged 23 commits into from
Nov 7, 2023

Conversation

BryanFauble
Copy link
Contributor

@BryanFauble BryanFauble commented Nov 6, 2023

Problem:

  • We want to automatically collect trace information for integration tests that are ran - allowing us to inspect this data without us manually running the tests on local machine.

Solution:

  • Created the aws config file and encrypted the values - it contains the PAT needed to get the AWS STS token to start a port forwarding session
  • Start a port-forward session in the CICD pipeline
  • Export trace data to the jaeger server running in the Sage EC2 service catalog
  • I re-worked how we determine if we are running integration tests to be earlier in the job
  • I added a run_opentelemetry boolean during the aws-check step

Testing:

  • I could not get this working on windows so I disabled it connecting to EC2
  • I verified that I could export trace data for both linux and macos:
    image
    image

@BryanFauble BryanFauble requested a review from a team as a code owner November 6, 2023 23:25
run: |
if [ -z "${{ secrets.encrypted_d17283647768_key }}" ] || [ -z "${{ secrets.encrypted_d17283647768_key }}" ]; then
echo "No test configuration decryption keys available, skipping integration tests"
# decrypt the encrypted test synapse configuration
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This all shows as changed because I removed the if check here - since I am doing it earlier in the steps.

The only notable change from this block is L175:

          if [ ${{ steps.aws-check.outputs.run_opentelemetry }} == "true" ]; then
            export SYNAPSE_OTEL_INTEGRATION_TEST_EXPORTER="otlp"
          fi

Copy link
Member

@zaro0508 zaro0508 left a comment

Choose a reason for hiding this comment

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

This is neat however the GH workflow is pretty complex, would it make sense to create a custom GH action for it?

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@BryanFauble
Copy link
Contributor Author

This is neat however the GH workflow is pretty complex, would it make sense to create a custom GH action for it?

@zaro0508 Maybe - I had originally intended to use https://github.com/marketplace/actions/aws-ssm-port-forwarding-session - But I tried it in this pipeline and I couldn't get trace data to export properly, so I fell back to doing it myself

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 nice - I'm going to pre-approve, as I just had a couple comments.

openssl aes-256-cbc -K ${{ secrets.encrypted_d17283647768_key }} -iv ${{ secrets.encrypted_d17283647768_iv }} -in test.synapseConfig.enc -out test.synapseConfig -d
mv test.synapseConfig ~/.synapseConfig
docker build -t sftp_tests - < tests/integration/synapseclient/core/upload/Dockerfile_sftp
docker run -d sftp_tests:latest
Copy link
Member

Choose a reason for hiding this comment

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

out of scope of this review, but did we hear back from Xa about the stfp server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't heard anything additional

.github/workflows/build.yml Outdated Show resolved Hide resolved
@BryanFauble BryanFauble merged commit f1b2a2f into develop Nov 7, 2023
35 checks passed
@BryanFauble BryanFauble deleted the SYNPY-1305-collect-trace-data-from-int-tests branch November 7, 2023 19:15
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.

None yet

3 participants