-
Notifications
You must be signed in to change notification settings - Fork 362
refactor(remote_config): move capability registration to feature modules #7127
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
Conversation
f62707b to
84300df
Compare
Overall package sizeSelf size: 4.37 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 |
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2026-01-08 13:43:04 Comparing candidate commit 43e0e1e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 288 metrics, 32 unstable metrics. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7127 +/- ##
==========================================
+ Coverage 84.53% 84.54% +0.01%
==========================================
Files 528 529 +1
Lines 22538 22542 +4
==========================================
+ Hits 19052 19058 +6
+ Misses 3486 3484 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
84300df to
918975b
Compare
8023afd to
05e2209
Compare
137abe9 to
041ef5b
Compare
05e2209 to
6b3bd2e
Compare
152a63b to
408e3f5
Compare
6b3bd2e to
69d7527
Compare
69d7527 to
4209c81
Compare
8171673 to
e501e3b
Compare
4209c81 to
c12a2a8
Compare
40b1dcb to
6abe01f
Compare
a8d37ec to
df5f680
Compare
6abe01f to
217d033
Compare
df5f680 to
8d1145b
Compare
6dba9c6 to
2d7b45a
Compare
8d1145b to
d1b1394
Compare
2d7b45a to
025cb92
Compare
d1b1394 to
0ca9227
Compare
025cb92 to
d9da38e
Compare
5be2cd4 to
d054943
Compare
d9da38e to
975b200
Compare
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)
d054943 to
85edbfa
Compare
975b200 to
a3b1125
Compare
| } | ||
| } | ||
|
|
||
| request(this.getPayload(), options, (err, data, statusCode) => { |
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.
Should we cache this already? Potentially add a TODO entry for that?
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.
Do you mean cache the result of this.getPayload(), or? Then we need to deal with cache invalidation as it can change from request to request, but it's doable. But I would say outside the scope of this PR as this code is just moved from the old packages/dd-trace/src/remote_config/manager.js file. I've added a TODO for now
…les (#7127) 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)
…les (#7127) 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)

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:
config/remote_config.jsfor client library config capabilities (APM_TRACING_*)openfeature/remote_config.jsfor OpenFeature capabilities (FFE_FLAG_CONFIGURATION_RULES)RemoteConfigManager→RemoteConfigandremote_config/manager.js→remote_config/index.jsproxy.jsto instantiateRemoteConfigdirectly instead of through wrapper functionMotivation
Motivation
This refactoring establishes a clear pattern where each feature manages its own remote config capabilities and handlers together:
OpenFeature is an exception to this pattern: it always registers the
FFE_FLAG_CONFIGURATION_RULEScapability (required by parametric tests since v5.72.0), but only registers its handler whenconfig.experimental.flaggingProvider.enabledis 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:
File renames (no logic changes, just find/replace):
remote_config/manager.js→remote_config/index.jstest/remote_config/manager.spec.js→test/remote_config/index.spec.jsRemoteConfigManager→RemoteConfig(throughout)New files with moved code:
config/remote_config.js- client library config capabilities moved from oldremote_config/index.js+APM_TRACINGhandler moved fromproxy.jsopenfeature/remote_config.js- FFE capability + handler moved fromproxy.jsModified files:
proxy.js- Removed old wrapper, now instantiatesRemoteConfigdirectly; handlers moved to feature modulestest/proxy.spec.js- Updated imports and assertions to match new structureAssertion lib:
chaiwithnode:assert/strictin remote_config tests for consistencyNo functional changes to remote config behavior, purely structural refactoring.