Skip to content

Conversation

@watson
Copy link
Contributor

@watson watson commented Jan 19, 2026

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_configs field represents the complete list of active configs, not incremental updates. When the tests sent separate APM_TRACING and LIVE_DEBUGGING payloads, previously applied configs would be unapplied if they weren't in the new client_configs list, causing the tests to fail unpredictably.

Changes

Core Fix:

  • Added send_rc_apm_tracing_and_probes() utility in utils/_remote_config.py to send both APM_TRACING and LIVE_DEBUGGING configs in a single RC command.

Test Improvements (tests/debugger/test_debugger_inproduct_enablement.py):

  • Fixed CO test to check all traces after a threshold, not just the first trace file
  • Added language-specific initial state handling (Node.js enables CO by default, others don't)

Manifest Updates:

  • Node.js: Enabled CO test starting from v5.83.0 (yet to be released to npm)
  • Java: Simplified CO manifest entry
  • Python: Simplified CO manifest entry
  • .NET: Simplified CO and DI manifest entry

Supporting Changes:

  • Updated scenario config to add faster RC polling and trace flushing intervals
  • Enhanced wait_for_code_origin_span() to use threshold-based filtering to avoid stale traces

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

Copy link
Contributor Author

watson commented Jan 19, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

CODEOWNERS have been resolved as:

manifests/dotnet.yml                                                    @DataDog/apm-dotnet @DataDog/asm-dotnet
manifests/java.yml                                                      @DataDog/asm-java @DataDog/apm-java
manifests/nodejs.yml                                                    @DataDog/dd-trace-js
manifests/python.yml                                                    @DataDog/apm-python @DataDog/asm-python
tests/debugger/test_debugger_inproduct_enablement.py                    @DataDog/debugger @DataDog/system-tests-core
tests/debugger/utils.py                                                 @DataDog/debugger @DataDog/system-tests-core
utils/_context/_scenarios/__init__.py                                   @DataDog/system-tests-core
utils/_remote_config.py                                                 @DataDog/system-tests-core

@watson watson force-pushed the watson/DEBUG-4402/fix-remote-enablement-code-origin-tests branch 2 times, most recently from 6304699 to 376a924 Compare January 19, 2026 20:57
@watson watson force-pushed the watson/DEBUG-4402/fix-remote-enablement-code-origin-tests branch 7 times, most recently from 09802dd to 29749db Compare January 21, 2026 10:40
@datadog-official
Copy link

datadog-official bot commented Jan 21, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 13c74bd | Docs | Datadog PR Page | Was this helpful? Give us feedback!

@watson watson force-pushed the watson/DEBUG-4402/fix-remote-enablement-code-origin-tests branch 2 times, most recently from b0b274e to 14e5970 Compare January 21, 2026 14:25
@watson watson marked this pull request as ready for review January 21, 2026 17:22
@watson watson requested review from a team as code owners January 21, 2026 17:22
@watson watson requested review from ZStriker19, daniel-romano-DD, jandro996 and rachelyangdog and removed request for a team January 21, 2026 17:22
@watson watson requested a review from mhlidd January 21, 2026 17:22
@watson
Copy link
Contributor Author

watson commented Jan 21, 2026

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.

@cbeauchesne
Copy link
Collaborator

Rebasing to fix the PHP issue.

@watson watson force-pushed the watson/DEBUG-4402/fix-remote-enablement-code-origin-tests branch from 2f3dc01 to 499435c Compare January 23, 2026 15:47
@cbeauchesne
Copy link
Collaborator

you'll need to rebase again to fix python issue (sorry)

Copy link
Collaborator

@nccatoni nccatoni left a 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

@nccatoni nccatoni self-requested a review January 26, 2026 10:30
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.
@watson watson force-pushed the watson/DEBUG-4402/fix-remote-enablement-code-origin-tests branch from 173745e to 489fd6c Compare January 26, 2026 12:20
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
Copy link
Member

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

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 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

Copy link
Member

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?

Copy link
Collaborator

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

Copy link
Contributor Author

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

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.

5 participants