refactor(config): introduce namespace field and start pruning internalPropertyName#8943
Conversation
Overall package sizeSelf size: 6.36 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: d85ba85 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-26 16:28:29 Comparing candidate commit d85ba85 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2262 metrics, 21 unstable metrics.
|
068f95d to
9c1be85
Compare
b66d304 to
78ff229
Compare
A `namespace` nests a configuration's canonical env name under a property path (`telemetry.DD_TELEMETRY_DEBUG`), so the public config object groups env-named leaves without the bespoke full paths `internalPropertyName` carried. It takes precedence over `configurationNames` when resolving the runtime property path and coexists with `internalPropertyName` so entries migrate one group at a time.
…mespace The runtime path moves from `remoteConfig.enabled` to `remoteConfig.DD_REMOTE_CONFIGURATION_ENABLED`. Telemetry still reports the canonical `DD_REMOTE_CONFIGURATION_ENABLED` name, since the canonical name is unaffected by how the property path is derived.
…spaces `appsec.apiSecurity.*` and `appsec.sca.enabled` move to the `appsec.apiSecurity`/`appsec.sca` namespaces, so the leaf becomes the canonical env name (`appsec.apiSecurity.DD_API_SECURITY_SAMPLE_DELAY`). The public `appsec.apiSecurity.enabled` key is unaffected.
The seven `telemetry.*` configs move to the `telemetry` namespace, so each leaf becomes its canonical env name (`telemetry.DD_TELEMETRY_HEARTBEAT_INTERVAL`). The telemetry protocol's own `heartbeatInterval` message field is unrelated and keeps its name.
…quire
The integration-test fixture apps require('dd-trace') the way a customer
does, so dd-trace is never in their own manifest. The rule stayed silent
on a clean install where dd-trace is unresolvable, but fired the moment
dd-trace became resolvable locally (yarn link), unlike CI. Disabling it
alongside the import/no-extraneous-dependencies rule already off for these
files mirrors the existing Next.js fixture exemption.
…amespaces The langchain/openai/vertexai span-char-limit and prompt-completion sample-rate configs now nest under their integration namespace, keyed by the canonical env-var name. makeUtilities derives those leaf names from the integration name, so the name passed in must match the env-var prefix (langchain -> DD_LANGCHAIN_*).
…namespace DD_API_SECURITY_MAX_DOWNSTREAM_BODY_BYTES was the only api-security entry still keyed by its legacy property name; nest it under the appsec.apiSecurity namespace like its siblings. The namespace object is always created from the defaults, so the optional chaining and the `?? 0` fallback on these accesses guarded a state that cannot occur; drop them.
…aces DD_PROFILING_ENABLED and DD_TRACE_STATS_COMPUTATION_ENABLED were stored under the legacy property names profiling.enabled and stats.enabled. Move them under their single-level namespaces so the canonical name is the key, like the other migrated groups. The programmatic `profiling` / `stats` options still route to these paths via configurationNames, so the public option surface is unchanged. span_stats.js reads the stats toggle via destructuring, so that consumer (and its fixtures) move with it. Restore the optional chaining on the api-security endpoint-collection accesses in telemetry/endpoints.js: those entries are not part of this migration, and the telemetry tests legitimately drive them with configs that omit appsec.
DD_LLMOBS_ENABLED was stored as llmobs.enabled; move it under the llmobs namespace so the canonical name is the key. The deprecated programmatic llmobs.enable()/disable() paths flip the same property and move with it. Left untouched: llmobs/telemetry.js still reads config._env['llmobs.enabled'], but config._env is never assigned on Config, so that read is already dead and its key is moot. Flagged for a separate cleanup rather than widening this diff.
…mespace DD_IAST_SECURITY_CONTROLS_CONFIGURATION was stored as iast.securityControlsConfiguration. Move it under the iast namespace so the canonical name is the key. v5 still exposes this through the (experimental) iast.* programmatic API, which major-overrides.js wires up. Keep the namespace in both majors and point the v5 configurationNames at the same canonical property, so the resolved path is iast.DD_IAST_SECURITY_CONTROLS_CONFIGURATION regardless of major and the consumer reads one key. The programmatic input names (which v6 rejects as unknown options) are unchanged.
Three readers still used the pre-namespace leaf names after the toggles moved under their namespace objects: 1. span_processor read `config.stats.enabled` instead of `stats.DD_TRACE_STATS_COMPUTATION_ENABLED`, so trace-stats computation never started once the property was renamed. 2. startup-log read `config.profiling.enabled` instead of `profiling.DD_PROFILING_ENABLED`, so `profiling_enabled` in the startup log was always false. 3. the profiling event serializer read `config.profiling.enabled` to derive the reported `activation`, so every profile reported `unknown` once the property was renamed; its near-OOM subprocess exporter built the same leaf and moves with it. Redundant optional chaining on these and the telemetry namespace reads is also dropped: a namespace object and its defaulted leaves are always present, so the guards only papered over test fixtures that built partial configs.
The disabled-path waf/rasp metric tests passed a partial `{ telemetry: {...} }`
literal, the same hand-built shape that drifted from the config and hid the
renamed-key regressions. Build them from `getConfig()` and flip the toggle under
test, matching the enabled-path beforeEach in the same files, so the fixtures
track the config's real shape instead of a duplicate of it.
endpoints.start now reads config.appsec.apiSecurity.endpointCollectionEnabled directly, after the optional chaining that only papered over partial test fixtures was dropped. The telemetry-logs fixture still built appsec without apiSecurity, so telemetry.start threw a TypeError and aborted the whole AppSec suite. Build it from a realistic config with endpoint collection disabled, matching the fixtures already migrated in the parent commit.
api-security and sca values now sit directly under `appsec` with their canonical env-name leaves rather than nested `apiSecurity`/`sca` objects. The three api-security entries keep their `configurationNames` so the public `appsec.apiSecurity.*` options still resolve after the runtime path moves to `config.appsec.DD_API_SECURITY_*`.
…al key
This makes the format-with-stats variant exercise stats again. SpanProcessor
reads config.stats.DD_TRACE_STATS_COMPUTATION_ENABLED since the toggle moved
under its namespace, so the hand-built `stats: { enabled }` left the
SpanStatsProcessor uncreated and the variant silently measured the no-stats
path -- an A/B "improvement" that was really lost coverage. The empty appsec
object is required too, since the same guard now reads config.appsec.standalone
without optional chaining.
Drive-by fix:
* Align llmobs-span-processor's config literal with the renamed DD_LLMOBS_ENABLED leaf.
…namespaces config.remoteConfig and config.iast are populated from defaults on every Config, so the `remoteConfig?.` guard in the express/fastify code-origin plugins and the `iastConfig?.` guard in the IAST security-controls reader only tolerated hand-built partial configs in tests. Read the namespace directly; the optional value leaf (DD_IAST_SECURITY_CONTROLS_CONFIGURATION) keeps its check.
… stats The preflight only checked that the 30-span chunk was exported, so a renamed stats toggle silently downgraded format-with-stats to the no-stats path and the loss surfaced as a spurious A/B improvement rather than a failure. Assert the stats processor presence matches WITH_STATS so config drift fails the bench instead of quietly changing what it measures.
Keep the benchmarks compatible while the config rename is in flight so they pass against either side of the nested-name transition.
…g keys The telemetry refactor renamed config.telemetry.enabled and config.telemetry.metrics to DD_INSTRUMENTATION_TELEMETRY_ENABLED and DD_TELEMETRY_METRICS_ENABLED. The sibling specs (rasp, waf, user) were updated but api_security.spec.js was missed, so enable() always received falsy flags and the count stub was never called.
780f8c7 to
d85ba85
Compare
|
As discussed: if there are any other requests to change things, we can follow up afterwards |
The sampling tests in #9030 build their own taggers with `{ llmobs: { enabled: true } }`, and #8943 renamed that config key to `DD_LLMOBS_ENABLED` everywhere it could see. The two landed in parallel, so #8943 normalized the rest of the file but never saw these four fixtures. On master the tagger now reads `DD_LLMOBS_ENABLED`, finds it undefined, and returns before registering the span; `Tagger.tagMap.get` then yields undefined and the "DROPPED at sampleRate 0" test throws synchronously, aborting the whole `test:llmobs:sdk:ci` run with exit 7. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28265509637/job/83751636644
…lPropertyName (#8943) `internalPropertyName` carried a hand-maintained full property path (`telemetry.debug`, `remoteConfig.enabled`) that diverged from the canonical env name, so the same configuration was named twice and the two could drift. A new `namespace` field nests the canonical env name under a property path (`telemetry.DD_TELEMETRY_DEBUG`), so the runtime path is derived from the canonical name plus a namespace with no separate alias to maintain. It takes precedence over `configurationNames` and `internalPropertyName` when resolving the path, in the eslint sync rule, and in the type generator. Every group of entries (remoteConfig, telemetry, appsec api-security and sca, profiling, stats, llmobs, iast security-controls, the per-integration llm span limits) moves onto it, and their runtime consumers are updated to the renamed keys. The canonical name telemetry reports is unchanged: the rename only affects how the property path is derived, not which env name is sent. The namespace object is always built from the defaults, so the optional chaining and `?? 0` fallbacks on the api-security accesses guarded a state that cannot occur and are dropped. Drive-by fix: * Exempt integration-test fixture apps from `n/no-extraneous-require`: they `require('dd-trace')` as a customer does, so the rule fired once dd-trace became locally resolvable (yarn link) but stayed silent on a clean install.
The sampling tests in #9030 build their own taggers with `{ llmobs: { enabled: true } }`, and #8943 renamed that config key to `DD_LLMOBS_ENABLED` everywhere it could see. The two landed in parallel, so #8943 normalized the rest of the file but never saw these four fixtures. On master the tagger now reads `DD_LLMOBS_ENABLED`, finds it undefined, and returns before registering the span; `Tagger.tagMap.get` then yields undefined and the "DROPPED at sampleRate 0" test throws synchronously, aborting the whole `test:llmobs:sdk:ci` run with exit 7. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28265509637/job/83751636644
…lPropertyName (#8943) `internalPropertyName` carried a hand-maintained full property path (`telemetry.debug`, `remoteConfig.enabled`) that diverged from the canonical env name, so the same configuration was named twice and the two could drift. A new `namespace` field nests the canonical env name under a property path (`telemetry.DD_TELEMETRY_DEBUG`), so the runtime path is derived from the canonical name plus a namespace with no separate alias to maintain. It takes precedence over `configurationNames` and `internalPropertyName` when resolving the path, in the eslint sync rule, and in the type generator. Every group of entries (remoteConfig, telemetry, appsec api-security and sca, profiling, stats, llmobs, iast security-controls, the per-integration llm span limits) moves onto it, and their runtime consumers are updated to the renamed keys. The canonical name telemetry reports is unchanged: the rename only affects how the property path is derived, not which env name is sent. The namespace object is always built from the defaults, so the optional chaining and `?? 0` fallbacks on the api-security accesses guarded a state that cannot occur and are dropped. Drive-by fix: * Exempt integration-test fixture apps from `n/no-extraneous-require`: they `require('dd-trace')` as a customer does, so the rule fired once dd-trace became locally resolvable (yarn link) but stayed silent on a clean install.
The sampling tests in #9030 build their own taggers with `{ llmobs: { enabled: true } }`, and #8943 renamed that config key to `DD_LLMOBS_ENABLED` everywhere it could see. The two landed in parallel, so #8943 normalized the rest of the file but never saw these four fixtures. On master the tagger now reads `DD_LLMOBS_ENABLED`, finds it undefined, and returns before registering the span; `Tagger.tagMap.get` then yields undefined and the "DROPPED at sampleRate 0" test throws synchronously, aborting the whole `test:llmobs:sdk:ci` run with exit 7. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28265509637/job/83751636644
…lPropertyName (#8943) `internalPropertyName` carried a hand-maintained full property path (`telemetry.debug`, `remoteConfig.enabled`) that diverged from the canonical env name, so the same configuration was named twice and the two could drift. A new `namespace` field nests the canonical env name under a property path (`telemetry.DD_TELEMETRY_DEBUG`), so the runtime path is derived from the canonical name plus a namespace with no separate alias to maintain. It takes precedence over `configurationNames` and `internalPropertyName` when resolving the path, in the eslint sync rule, and in the type generator. Every group of entries (remoteConfig, telemetry, appsec api-security and sca, profiling, stats, llmobs, iast security-controls, the per-integration llm span limits) moves onto it, and their runtime consumers are updated to the renamed keys. The canonical name telemetry reports is unchanged: the rename only affects how the property path is derived, not which env name is sent. The namespace object is always built from the defaults, so the optional chaining and `?? 0` fallbacks on the api-security accesses guarded a state that cannot occur and are dropped. Drive-by fix: * Exempt integration-test fixture apps from `n/no-extraneous-require`: they `require('dd-trace')` as a customer does, so the rule fired once dd-trace became locally resolvable (yarn link) but stayed silent on a clean install.
The sampling tests in #9030 build their own taggers with `{ llmobs: { enabled: true } }`, and #8943 renamed that config key to `DD_LLMOBS_ENABLED` everywhere it could see. The two landed in parallel, so #8943 normalized the rest of the file but never saw these four fixtures. On master the tagger now reads `DD_LLMOBS_ENABLED`, finds it undefined, and returns before registering the span; `Tagger.tagMap.get` then yields undefined and the "DROPPED at sampleRate 0" test throws synchronously, aborting the whole `test:llmobs:sdk:ci` run with exit 7. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28265509637/job/83751636644
Summary
internalPropertyNamecarries a bespoke full property path (telemetry.debug,remoteConfig.enabled) that diverges from the canonical env name. This adds anamespacefield that nests the canonical env name under a property path(
remoteConfig.DD_REMOTE_CONFIGURATION_ENABLED) and migrates entries onto it sothe property path is derivable from the canonical name plus a namespace, with no
hand-maintained alias.
namespaceresolves the runtime property path and takes precedence overconfigurationNames; it coexists withinternalPropertyNameso entries moveone group at a time and the final commit drops
internalPropertyName.remoteConfig.enabledis migrated as the first entry. Telemetry still reportsthe canonical
DD_REMOTE_CONFIGURATION_ENABLEDname — the canonical name isindependent of how the property path is derived.
Remaining work
DD_API_KEY, the CI-visibility flags,DD_TRACE_ENABLED, query-string obfuscation, resource renaming, scope) — dropinternalPropertyName, fall back to the canonical name.telemetry.*(7 entries) — high test-fixture churn: many specs buildtelemetry: { enabled, metrics, ... }literals.appsec.*(apiSecurity.*,sca.enabled).iast.securityControlsConfiguration— has a v5 override inmajor-overrides.js; keep both majors resolving the same runtime path.llmobs.enabledand the AI-lib span limits (langchain/openai/vertexai).profiling.enabled,stats.enabled— namespace takes precedence over the public programmatic name.internalPropertyNamefrom the typedef,defaults.js,generate-config-types.js, and the eslint rule once nothing uses it.Open question
The AI-lib span limits share a generic consumer:
plugins/util/llm.jsdestructures{ spanCharLimit, spanPromptCompletionSampleRate }off whichever integrationconfig it is handed. The namespace shape gives each library a distinct canonical
leaf (
DD_LANGCHAIN_SPAN_CHAR_LIMITvsDD_OPENAI_SPAN_CHAR_LIMIT), so the shareddestructure no longer works without per-library keys. Worth deciding whether those
keep a uniform leaf or the helper learns the per-library names.
Test plan
packages/dd-trace/test/config/index.spec.jspackages/dd-trace/test/proxy.spec.jsnode scripts/generate-config-types.js --checkremoteConfiggate)