Skip to content

Conversation

@watson
Copy link
Collaborator

@watson watson commented Dec 17, 2025

What does this PR do?

Refactors remote config capability and handler registration to improve separation of concerns. Moves feature-specific remote config setup from a central location into their respective feature modules, following the same pattern already used by AppSec.

Key changes:

  • Creates config/remote_config.js for client library config capabilities (APM_TRACING_*)
  • Creates openfeature/remote_config.js for OpenFeature capabilities (FFE_FLAG_CONFIGURATION_RULES)
  • Renames RemoteConfigManagerRemoteConfig and remote_config/manager.jsremote_config/index.js
  • Simplifies proxy.js to instantiate RemoteConfig directly instead of through wrapper function

Motivation

Motivation

This refactoring establishes a clear pattern where each feature manages its own remote config capabilities and handlers together:

  • Core features (like APM tracing) → always registered
  • Optional features (like AppSec) → registered only when feature is enabled

OpenFeature is an exception to this pattern: it always registers the FFE_FLAG_CONFIGURATION_RULES capability (required by parametric tests since v5.72.0), but only registers its handler when config.experimental.flaggingProvider.enabled is true. This hybrid approach satisfies test requirements while keeping the experimental feature properly gated.

Additional Notes

Reviewing this PR:
The diff is large but mostly consists of code being moved between files. To review more easily:

  1. File renames (no logic changes, just find/replace):

    • remote_config/manager.jsremote_config/index.js
    • test/remote_config/manager.spec.jstest/remote_config/index.spec.js
    • Class RemoteConfigManagerRemoteConfig (throughout)
  2. New files with moved code:

    • config/remote_config.js - client library config capabilities moved from old remote_config/index.js + APM_TRACING handler moved from proxy.js
    • openfeature/remote_config.js - FFE capability + handler moved from proxy.js
  3. Modified files:

    • proxy.js - Removed old wrapper, now instantiates RemoteConfig directly; handlers moved to feature modules
    • test/proxy.spec.js - Updated imports and assertions to match new structure
  4. Assertion lib:

    • Replaced chai with node:assert/strict in remote_config tests for consistency

No functional changes to remote config behavior, purely structural refactoring.

Copy link
Collaborator Author

watson commented Dec 17, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@github-actions
Copy link

github-actions bot commented Dec 17, 2025

Overall package size

Self size: 4.36 MB
Deduped: 5.19 MB
No deduping: 5.19 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@datadog-official

This comment has been minimized.

@pr-commenter
Copy link

pr-commenter bot commented Dec 17, 2025

Benchmarks

Benchmark execution time: 2025-12-19 19:13:03

Comparing candidate commit 408e3f5 in PR branch watson/DEBUG-4402/even-more-rc-split with baseline commit 69d7527 in branch watson/DEBUG-4402/split-rc-and-appsec.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 292 metrics, 28 unstable metrics.

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 97.19626% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.82%. Comparing base (69d7527) to head (408e3f5).

Files with missing lines Patch % Lines
packages/dd-trace/src/remote_config/index.js 97.34% 5 Missing ⚠️
packages/dd-trace/src/config/remote_config.js 92.30% 1 Missing ⚠️
Additional details and impacted files
@@                            Coverage Diff                            @@
##           watson/DEBUG-4402/split-rc-and-appsec    #7127      +/-   ##
=========================================================================
- Coverage                                  84.82%   84.82%   -0.01%     
=========================================================================
  Files                                        523      524       +1     
  Lines                                      22213    22217       +4     
=========================================================================
+ Hits                                       18842    18845       +3     
- Misses                                      3371     3372       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@watson watson force-pushed the watson/DEBUG-4402/even-more-rc-split branch from 84300df to 918975b Compare December 18, 2025 08:36
@watson watson force-pushed the watson/DEBUG-4402/split-rc-and-appsec branch from 8023afd to 05e2209 Compare December 18, 2025 08:36
@watson watson force-pushed the watson/DEBUG-4402/even-more-rc-split branch 2 times, most recently from 137abe9 to 041ef5b Compare December 18, 2025 11:25
@watson watson force-pushed the watson/DEBUG-4402/split-rc-and-appsec branch from 05e2209 to 6b3bd2e Compare December 19, 2025 17:38
@watson watson force-pushed the watson/DEBUG-4402/even-more-rc-split branch 2 times, most recently from 573563d to 152a63b Compare December 19, 2025 18:05
Move remote config capability and handler registration from central location
to feature-specific modules for better separation of concerns.

Changes:
- Create tracing/remote_config.js for APM_TRACING_* capabilities and handler
- Create openfeature/remote_config.js for FFE_FLAG_CONFIGURATION_RULES capability
- Remove remote_config/index.js wrapper, instantiate RemoteConfig directly in proxy
- Rename RemoteConfigManager class to RemoteConfig
- Rename remote_config/manager.js to remote_config/index.js
- Replace chai with node:assert/strict in remote_config tests
- Use real implementations in proxy tests instead of stubbing

This creates a consistent pattern where features own their remote config setup:
- Core APM tracing → tracing/remote_config.js (always enabled)
- OpenFeature → openfeature/remote_config.js (conditional)
- AppSec → appsec/remote_config.js (conditional)
@watson watson force-pushed the watson/DEBUG-4402/even-more-rc-split branch from 152a63b to 408e3f5 Compare December 19, 2025 19:02
@watson watson force-pushed the watson/DEBUG-4402/split-rc-and-appsec branch from 6b3bd2e to 69d7527 Compare December 19, 2025 19:02
@watson watson requested review from a team, BridgeAR and simon-id December 19, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants