-
Notifications
You must be signed in to change notification settings - Fork 14
[python]: enable forwarder tests v2 #4924
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
base: main
Are you sure you want to change the base?
Conversation
This commit accounts for the two events we expect, one from the injector and one from the tracer so that we can enable these tests for Python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this lgtm, and Mark and I ran these on a video call with an image with the change from dd-trace-py and both tests are passing, we also tested with the 3.10.2 artifact to confirm it is skipped, and they are skipped for older versions of Python we do not support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forced to test with the latest python tracer, and the tests are failing:
weblog_url:
agent: 1:7.68.0-1
datadog-apm-inject: 0.39.1-dev.b0d6e40.glci945052743.ga97db900
datadog-apm-library: 3.11.0.dev82+g378df652a
docker:
datadog-installer: 7.63.0-installer-0.12.14-rc.3
Configuration
app_type: docker_ssi
os: Ubuntu_22_amd64
arch: amd64
runtime_version: 3.8.20
Pulling ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.20.0
https://gitlab.ddbuild.io/DataDog/system-tests/-/pipelines/70267782
Let me know if you need help to solve this
@robertomonteromiguel this change depends on DataDog/dd-trace-py#13926 being merged first with the fix. So the fact that this currently fails on latest is known "blocker" 👍🏻 We haven't found a good way to merge this change first, so we can ensure it runs on the PR in dd-trace-py before merging the dd-trace-py PR. So we were going to merge the dd-trace-py PR first, and then this one (not ideal, but... do we have other options?) |
@brettlangdon you could add the variable "SYSTEM_TESTS_REF" in the gitlab file of your python PR (configure_system_tests) and point to the latest commit hash of this system-tests PR. |
Motivation
Python has been failing the forwarder tests for some time. The injection metadata tests added by @sydney-tung also rely on this feature to be working. I've fixed the Python tracer integration with telemetry forwarding here: DataDog/dd-trace-py#13926. This change enables both tests so they can be run on the Python pipeline.
Changes
This change enables the two broken forwarder tests for python that have been fixed, which unblocks the addition of the injection metadata test.
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
[<language>]
, double-check that only<language>
is impacted by the changebuild-XXX-image
label is present