-
Notifications
You must be signed in to change notification settings - Fork 14
fix(debugger): enable code origin in-product enablement tests #6063
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?
fix(debugger): enable code origin in-product enablement tests #6063
Conversation
|
|
6304699 to
376a924
Compare
09802dd to
29749db
Compare
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: 13c74bd | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
b0b274e to
14e5970
Compare
|
The currently failing tests are unrelated to this PR and actively being fixed in other PRs. The relevant tests to this PR are currently all passing. |
|
Rebasing to fix the PHP issue. |
2f3dc01 to
499435c
Compare
|
you'll need to rebase again to fix python issue (sorry) |
nccatoni
left a comment
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.
LGTM but you should get a review from someone familiar with the feature
The code origin in-product enablement tests were previously skipped by all
languages due to underlying issues with Remote Config interaction. This commit
fixes those issues and enables the tests.
1. **Root Cause**: Remote Config's client_configs represents the COMPLETE list
of active configs, not incremental updates. Sending separate APM_TRACING and
LIVE_DEBUGGING payloads caused previously applied configs to be unapplied
when they weren't in the new client_configs list, making the tests unreliable.
2. **Solution**: Introduced send_rc_apm_tracing_and_probes() utility to send
both APM_TRACING and LIVE_DEBUGGING configs in a single command, preventing
unintended unapplication of configs.
3. **Test Improvements**:
- Added timing controls (RC_POLL_INTERVAL, TRACE_FLUSH_WAIT) for reliable
config polling and trace flushing
- Fixed code origin test to check all traces after threshold, not just one
- Added language-specific initial state handling (Node.js enables code origin
by default)
- Reduced environment setup to minimal required config
- Removed decorator-based skip/bug markings in favor of manifest declarations
4. **Manifest Updates**:
- Node.js: Enabled code origin test starting from v5.83.0 (first time!)
- Java: Code origin remains skipped (missing_feature, simplified manifest)
- Python: Code origin remains skipped (missing_feature, simplified manifest)
- .NET: Moved bug marking (DEBUG-4637) from test decorators to manifest
These changes enable code origin in-product enablement tests for Node.js and
establish the correct testing infrastructure for other languages to follow.
173745e to
489fd6c
Compare
| spring-boot-undertow: v1.48.0 | ||
| spring-boot-wildfly: v1.48.0 | ||
| uds-spring-boot: v1.48.0 | ||
| tests/debugger/test_debugger_inproduct_enablement.py::Test_Debugger_InProduct_Enablement_Code_Origin: missing_feature |
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.
When you define it as missing_feature, does it retain the force_skip=True? These tests take quite a bit of time so we'd like to make sure they don't run unless they're passing
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 think that is outside the scope of this PR due to new rules. For details of this change, see this discussion: #6063 (comment), and this PR: #6059
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.
cc @nccatoni @cbeauchesne Do the manifests support force_skip yet? I thought we were still supposed to rely on decorators until they did?
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.
@nccatoni is implemeting it. It won't be inside manifest because it's not related to version declaration, but rather a direct property of method classes. But it'll achieve the same
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.
Is it a blocker for this PR though, or can we clean it up later once the new functionality is added? A lot of force-skip was already removed in the PR I mentioned above

Motivation
The code origin (CO) in-product enablement tests were never actually working - they were skipped by Java, Python, and .NET, and not enabled by Node.js. Investigation revealed a fundamental issue with how the tests were interacting with Remote Config (RC) that made them unreliable.
RC's
client_configsfield represents the complete list of active configs, not incremental updates. When the tests sent separateAPM_TRACINGandLIVE_DEBUGGINGpayloads, previously applied configs would be unapplied if they weren't in the newclient_configslist, causing the tests to fail unpredictably.Changes
Core Fix:
send_rc_apm_tracing_and_probes()utility inutils/_remote_config.pyto send bothAPM_TRACINGandLIVE_DEBUGGINGconfigs in a single RC command.Test Improvements (
tests/debugger/test_debugger_inproduct_enablement.py):Manifest Updates:
Supporting Changes:
wait_for_code_origin_span()to use threshold-based filtering to avoid stale tracesWorkflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present