Use stable config for RUM snippet creation#331
Conversation
- Auto path: try_build_snippet_from_env now uses snippet_create_from_stable_config to read from application_monitoring.yaml instead of building JSON from DD_RUM_* env vars - Directive path: on_datadog_rum_config seeds from stable config via stable_config_get_entries, with directive block values overriding - Trim env var forwarding to only DD_RUM_ENABLED and DD_RUM_APPLICATION_ID - Pin inject-browser-sdk to 587b071 (includes stable_config_get_entries FFI)
A transitive dependency (clap_builder 4.6.0, via libdd-library-config) now requires the Rust 2024 edition which needs Rust 1.85+.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #331 +/- ##
=======================================
Coverage 68.85% 68.85%
=======================================
Files 56 56
Lines 7471 7471
Branches 1058 1058
=======================================
Hits 5144 5144
Misses 1820 1820
Partials 507 507 🚀 New features to boost your workflow:
|
The :latest tag was cached by nydus snapshotter on CI runners. Use an explicit tag to force pulling the updated image with Rust 1.85.0.
aws-lc-sys (transitive dep via libdd-library-config -> rustls) compiles C code during its build script. Without sysroot flags, the linker cannot find crtbeginS.o in the musl cross-compilation environment.
Rust_CARGO_TARGET is empty at the point corrosion_set_env_vars runs. Use hardcoded x86_64/aarch64 target names instead — only one will match the actual cargo build target, the other is harmlessly ignored.
aws-lc-sys build script compile+links test binaries via clang. Without these flags, clang invokes system GNU ld which can't find crtbeginS.o and libgcc in the musl sysroot. Using lld and compiler-rt matches the rest of the toolchain setup.
test_env_only_config and test_partial_env_config now write application_monitoring.yaml into the container instead of setting DD_RUM_* env vars, matching the new stable config integration.
test_env_remote_configuration_id, test_env_disabled_overrides_env_config, and test_env_disabled_location_override now use application_monitoring.yaml instead of DD_RUM_* env vars. DD_RUM_ENABLED still uses env var since it controls the enable/disable flag, not the config source.
…hared_library_for_env # Conflicts: # .gitlab/build-and-test-fast.yml # .gitlab/common.yml # .gitlab/prepare-oci-package.sh # .gitlab/ssi-package.yml
Adds DD_RUM_REMOTE_CONFIGURATION_ID support in configuration_from_getter so remoteConfigurationId appears in snippets created via stable config.
inject-browser-sdk gained a 3-arg snippet_create_from_stable_config (language, debug_logs, json_overlay) that reads YAML and merges an optional JSON overlay internally. Move both call sites to it: - on_datadog_rum_config builds the overlay from block directives instead of pre-reading entries with the (now removed) stable_config_get_entries. - try_build_snippet_from_env passes a NULL overlay. Switch inject-browser-sdk back to a git submodule (deps/inject-browser-sdk), add_subdirectory()d so its CMakeLists handles corrosion + cbindgen. Bump build_env rust toolchain to 1.94.0 to match the SDK's rust-toolchain. Add ngx_pcalloc stub so the catch2 unit_tests target links, and update EXPECTED_BIG_CHONK to the SDK's new alphabetical key ordering.
These tests were originally written against DD_RUM_* environment variables, then migrated to read from the stable-config YAML, but their names and conf files were never updated. Rename for accuracy: - test_env_only_config -> test_stable_config_only - test_partial_env_config -> test_nginx_config_overlays_stable_config - test_env_remote_configuration_id -> test_stable_config_remote_configuration_id - test_env_disabled_overrides_env_config -> test_dd_rum_enabled_false_overrides_stable_config - test_env_disabled_location_override -> test_location_off_overrides_stable_config - rum_env_only.conf -> rum_stable_config_only.conf - rum_partial_env.conf -> rum_stable_config_partial.conf - <ENV_APP_ID> / <ENV_TOKEN> placeholders -> <STABLE_APP_ID> / <STABLE_TOKEN>
There was a problem hiding this comment.
Pull request overview
This PR switches RUM snippet generation to prefer Datadog Agent “stable config” (application_monitoring.yaml) via snippet_create_from_stable_config, and updates build/test plumbing to vendor inject-browser-sdk as a submodule and adjust CI/toolchain versions.
Changes:
- Update RUM config paths to generate snippets from stable config (with optional directive overlay) and reduce forwarded env vars.
- Refactor tests to write/read stable config inside the nginx test container and add new nginx configs for stable-config scenarios.
- Vendor
inject-browser-sdkas a submodule and adjust build/CI images and nginx version matrices.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/rum/config.cpp |
Switch snippet creation to stable-config API; adjust env var forwarding and directive overlay behavior. |
test/cases/rum/test_injection.py |
Replace env-only RUM tests with stable-config-based tests using container-written YAML. |
test/cases/rum/conf/rum_stable_config_only.conf |
New nginx config fixture for stable-config-only injection. |
test/cases/rum/conf/rum_stable_config_partial.conf |
New nginx config fixture for “stable config + directive overrides” behavior. |
test/unit/stub_nginx.c |
Add missing nginx allocator stub needed by updated config code paths. |
deps/CMakeLists.txt |
Switch from FetchContent to submodule-based build of inject-browser-sdk; add cross-compilation env wiring. |
.gitmodules |
Add deps/inject-browser-sdk submodule. |
build_env/Dockerfile |
Update Rust toolchain version used in the musl toolchain image. |
.gitlab/common.yml |
Pin build image tag to a Rust-specific toolchain image tag. |
.gitlab/ssi-package.yml |
Pin SSI packaging job image tag to Rust-specific toolchain image tag. |
.gitlab/build-and-test-fast.yml |
Pin shellcheck image tag and add nginx 1.29.5 to build/test matrices. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hared_library_for_env # Conflicts: # .gitlab/build-and-test-fast.yml # .gitlab/ssi-package.yml
- build_env/Dockerfile: replace Unicode en-dash in `–tlsv1.2` with
ASCII `--tlsv1.2` so curl parses the option
- src/rum/config.cpp:
- Rename try_build_snippet_from_env to try_build_snippet_from_stable_config
(it no longer reads env vars; the stable-config YAML is the source)
- Drop env-fallback telemetry tag block (DD_RUM_APPLICATION_ID lookup +
hardcoded remote_config_used:false). Telemetry tags from the snippet
path now leave fields empty when the nginx block doesn't supply them
instead of misreporting `false`.
- Drop hardcoded remote_config_used:false in apply_rum_config_tags so
stable-config-supplied remoteConfigurationId isn't mis-tagged.
- Remove DD_RUM_APPLICATION_ID from get_environment_variable_names —
it's no longer read.
The merge of origin/master kept this branch's newer dd-trace-cpp ref (f8c39137) over master's (92cdfe6). Roll back to master's pointer to keep this PR scoped to RUM stable-config changes.
clang-format wraps the snippet_create_from_stable_config call onto one line; yapf wraps Path(__file__) onto multiple lines so the conf paths fit within the line length.
system-tests builds with RUM=OFF, so the private inject-browser-sdk submodule is not needed. Init only dd-trace-cpp and libddwaf in CI to avoid the auth failure on the private repo. Also normalize the inject-browser-sdk URL to a relative path for consistency with the other submodules.
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 8682786 | Docs | Datadog PR Page | Give us feedback! |
- Remove get_rum_config_from_env, get_rum_enabled_from_env, rum_env_mappings and the env_mapping struct; only ever called from unit tests. - Remove the corresponding catch2 tests and their ScopedEnv helpers. - Factor the libdd-library-config "nginx" identifier into a rum_language constant in config_internal.h to remove the literal duplication. - Trim verbose comment in deps/CMakeLists.txt and the WHAT comment on the overlay-build line in config.cpp; keep only the WHY about the SDK merge.
The github source is private and requires PAT/SSH auth that CI runners don't have. The gitlab.ddbuild.io DataDog mirror is reachable via the GitLab CI job token, so use it as the submodule URL.
parse_bool used an inline lambda over std::tolower; switch to the
existing datadog::nginx::to_lower helper from src/string_util.h.
Drop the WHAT comments on the two SDK-validation fallbacks in
make_rum_json_config — the if/else structure already conveys the
meaning ("not a number → pass as string").
Use yaml.safe_dump and a _stable_config context manager to dedupe write/cleanup boilerplate across the five stable-config tests. Hoist imports to the top, extract the config path constant, drop unused captures, and add PyYAML to test deps.
Both _write_stable_config and _cleanup_stable_config had a single caller (the new context manager); fold them in. Also use Orchestration.nginx_replace_file for the write, which already handles the mkdir -p $(dirname ...) heredoc pattern instead of duplicating it.
Mirrors nginx_replace_file so _stable_config no longer needs raw subprocess/docker_compose_command/child_env plumbing for cleanup.
Restores the rum_remote_config_tag = "remote_config_used:false" default that was lost in the env-var->stable-config migration, ensuring telemetry never emits an empty tag value. Switches rum_language from string_view to const char* since it is passed to a C FFI that requires null termination. Also extracts a _read_conf test helper, refreshes stale "env vars" comments, and restores the "exception" catch-parameter name for codebase consistency.
- Extract add_member/add_first_as_string lambdas in make_rum_json_config - Add make_stable_config_snippet helper to wrap Snippet unique_ptr - Replace child_explicit/parent_explicit pair with single rum_enable_unset - Drop redundant inline comment in reload_nginx_with_empty_config
The CI test image bakes /.venv from a snapshot of test/pyproject.toml, refreshed only by a manual `make build-push-test-image`. New deps (PyYAML for the RUM stable-config tests) didn't reach CI. Switch to `uv run --project test test/bin/run.py ...` so each job syncs test/.venv against the source pyproject.toml before running.
The coverage job inherits the same yaml-import failure: it uses the build image (whose baked /.venv is also stale) and invokes run.py directly. Switch the Makefile's test/test-openresty/coverage targets to `uv run --project test` so they sync against the source pyproject.toml, matching the pattern just applied to .gitlab/common.yml.
|
Thanks @xavier-datadog |
Summary
Switches external configuration to stable config (file based config managed by the agent), which is used as baseline for RUM config.
Adopts inject-browser-sdk's new 3-arg
snippet_create_from_stable_config(language, debug, json_overlay)API. The SDK now readsapplication_monitoring.yamland merges an optional JSON overlay internally —on_datadog_rum_configbuilds the overlay from block directives;try_build_snippet_from_stable_configpasses NULL.inject-browser-sdk is vendored as a git submodule (
deps/inject-browser-sdk). Build toolchain bumped to rust 1.94 to match the SDK'srust-toolchain.toml. Misleadingtest_env_*integration tests renamed totest_stable_config_*(they exercise YAML, not env vars).