Add DD_RUM_* env var support for RUM configuration (#304)

* Add DD_RUM_* environment variable support for RUM configuration

Add support for configuring RUM via DD_RUM_* environment variables as
fallback defaults. Nginx `datadog_rum_config` directive values always
take precedence over env vars on a per-field basis. When no directive
exists and valid env vars are present, RUM auto-enables.

Environment variables: DD_RUM_ENABLED, DD_RUM_APPLICATION_ID,
DD_RUM_CLIENT_TOKEN, DD_RUM_SITE, DD_RUM_SERVICE, DD_RUM_ENV,
DD_RUM_VERSION, DD_RUM_SESSION_SAMPLE_RATE,
DD_RUM_SESSION_REPLAY_SAMPLE_RATE, DD_RUM_TRACK_RESOURCES,
DD_RUM_TRACK_LONG_TASKS, DD_RUM_TRACK_USER_INTERACTIONS,
DD_RUM_REMOTE_CONFIGURATION_ID.

Precedence rules:
- `datadog_rum on/off` always wins over DD_RUM_ENABLED
- `datadog_rum_config` fields always win over DD_RUM_* env vars
- DD_RUM_ENABLED=false explicitly disables even with config env vars
- Unrecognized DD_RUM_ENABLED values log a warning and skip auto-enable

Also includes:
- Case-insensitive parse_bool() for boolean env var parsing
- Fix std::stod crash with std::strtod for sample rate parsing
- Fix snippet memory leak on config error path
- Guard against rum_enable=1 with null snippet (prevents segfault)
- Null-terminated error messages via ngx_pcalloc
- Empty values vector guard in make_rum_json_config
- Use FetchContent for inject-browser-sdk dependency
- RUM integration tests and C++ unit tests
- RUM CI pipeline in GitLab

* Make rum_env_mappings constexpr and rename DD_RUM_VERSION to DD_RUM_MAJOR_VERSION

Move rum_env_mappings definition into config_internal.h as an
inline constexpr std::to_array, eliminating the separate
rum_env_mappings_size variable in favor of .size(). Rename
DD_RUM_VERSION to DD_RUM_MAJOR_VERSION.

* Use range-based for with structured bindings for rum_env_mappings loops

Replace index-based iteration with range-based for and structured
bindings. Also simplify std::getenv call to use string_view::data()
directly instead of constructing a temporary std::string.

* Extract conf_err() helper to combine error buffer allocation and formatting

Replace repeated alloc_err_buf + ngx_snprintf pairs with a single
conf_err() variadic template. Also fix "Rust SDK" comment to "RUM SDK"
and use static_cast for C-style casts.

* Name magic number 5 as default_rum_config_version constant

* Use RAII for Snippet lifetime and minor cleanups

Wrap snippet_create_from_json in unique_ptr with snippet_cleanup
deleter to ensure cleanup on all paths including exceptions. Also
rename catch variable from 'e' to 'exception'.

* Extract try_build_snippet_from_env() from merge_loc_config

Reduce nesting by moving the env-based snippet construction into its
own function with early returns.

* Extract resolve_rum_enable_from_env() from merge_loc_config

Move the DD_RUM_ENABLED resolution logic into its own function with
early returns to reduce nesting.

* Rename environment_variable_names to get_environment_variable_names

Use a verb prefix for the rum namespace function, and simplify
bool-to-ngx_flag_t conversion with static_cast.

* Fix env var name mismatch in tests and UB in conf_err call

Update tests and docker-compose to use DD_RUM_ENVIRONMENT and
DD_RUM_MAJOR_VERSION matching the mapping table. Convert string_view
to std::string before passing to variadic ngx_snprintf to avoid UB.

* Improve error handling and fix silent failures in RUM config

- Add NULL check in conf_err for ngx_pcalloc failure
- Log warning when json.empty() after env vars were read
- Rethrow std::bad_alloc instead of swallowing it; fix misleading
  catch message
- Pass unrecognized boolean values as strings to RUM SDK instead of
  silently defaulting to false
- Refactor resolve_rum_enable_from_env to read getenv once

* Fix RUM build: reinterpret_cast for u_char* and add string_view_literals

- Use reinterpret_cast instead of static_cast for char* to u_char*
  conversion in conf_err (clang rejects static_cast between unrelated types)
- Add `using namespace std::string_view_literals` so the `sv` suffix
  is available for the DD_RUM_ENABLED string literal

* Fix GHA build: allow overriding base image to bypass internal registry mirror

The GitHub Actions runners have registry.ddbuild.io configured as a Docker
mirror, but can't reach it. Parametrize the base image via ARG so GHA can
use the public alpine:3.20.3 instead.

* Address GitLab CI review comments: reorder and expand RUM jobs

- Move .build-nginx-rum template after .build-nginx in common.yml
- Group build-nginx-rum-fast with other build jobs in build-and-test-fast.yml
- Expand RUM version matrix to cover all major versions (1.24, 1.25, 1.26, 1.27, 1.28, 1.29)
- Move build-nginx-rum-all/test-nginx-rum-all inline with other jobs in build-and-test-all.yml
- Add missing 1.25.5 and 1.27.5 to the all-versions RUM matrix

* Address C++ review comments in rum/config.cpp

- Rename single-letter loop variable `e` to `entry`
- Extract duplicated tag-setting logic into apply_rum_config_tags() helper

* Address review comments: nits and consistency fixes

- Makefile: add space before $(if for readability
- config_internal.h: add rum_config_map type alias; use in signatures
- config.cpp: use values.front() instead of values[0] throughout
- config.cpp: fix misleading version error message (vN not v5)
- test_rum_config.cpp: group rapidjson include with other third-party includes
- test_injection.py: replace bare assert with self.assertXxx() for consistency

* Fix: restore arg_values[0] for ngx_str_t* pointer (not a vector)

Author: pawelchcki

.github/workflows/system-tests.yml

@@ -19,6 +19,7 @@ jobs:
       #   - https://github.com/DataDog/system-tests/blob/main/utils/scripts/load-binary.sh
       NGINX_VERSION: 1.29.5
       WAF: ON
+      BASE_IMAGE: alpine:3.20.3
     steps:
       - name: checkout
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0

.gitlab/build-and-test-all.yml

@@ -54,6 +54,22 @@ build-nginx-all:
           - "1.29.5"
         WAF: ["ON", "OFF"]
 
+build-nginx-rum-all:
+  extends:
+    - .build-all
+    - .build-nginx-rum
+  parallel:
+    matrix:
+      - ARCH: ["amd64", "arm64"]
+        NGINX_VERSION:
+          - "1.24.0"
+          - "1.25.5"
+          - "1.26.3"
+          - "1.27.5"
+          - "1.28.2"
+          - "1.29.5"
+        WAF: ["OFF"]
+
 build-ingress-nginx-all:
   extends:
     - .build-all
@@ -237,6 +253,37 @@ test-nginx-all-bis:
         NGINX_VERSION: ["1.24.0"]
         WAF: ["ON", "OFF"]
 
+test-nginx-rum-all:
+  extends:
+    - .test-all
+    - .test-nginx-rum
+  parallel:
+    matrix:
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.24.0"]
+        NGINX_VERSION: ["1.24.0"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.25.5"]
+        NGINX_VERSION: ["1.25.5"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.26.3"]
+        NGINX_VERSION: ["1.26.3"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.27.5"]
+        NGINX_VERSION: ["1.27.5"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.28.2"]
+        NGINX_VERSION: ["1.28.2"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.29.5"]
+        NGINX_VERSION: ["1.29.5"]
+        WAF: ["OFF"]
+
 test-ingress-nginx-all:
   extends:
     - .test-all
@@ -296,3 +343,4 @@ test-openresty-all:
           - "1.27.1.1"
           - "1.27.1.2"
         WAF: ["ON", "OFF"]
+

.gitlab/build-and-test-fast.yml

@@ -62,6 +62,22 @@ build-openresty-fast:
         RESTY_VERSION: ["1.27.1.2"]
         WAF: ["ON", "OFF"]
 
+build-nginx-rum-fast:
+  extends:
+    - .build-and-test-fast
+    - .build-nginx-rum
+  parallel:
+    matrix:
+      - ARCH: ["amd64", "arm64"]
+        NGINX_VERSION:
+          - "1.24.0"
+          - "1.25.5"
+          - "1.26.3"
+          - "1.27.5"
+          - "1.28.2"
+          - "1.29.5"
+        WAF: ["OFF"]
+
 test-nginx-fast:
   extends:
     - .build-and-test-fast
@@ -121,6 +137,38 @@ test-openresty-fast:
         RESTY_VERSION: ["1.27.1.2"]
         WAF: ["ON", "OFF"]
 
+test-nginx-rum-fast:
+  extends:
+    - .build-and-test-fast
+    - .test-nginx-rum
+  needs: [build-nginx-rum-fast]
+  parallel:
+    matrix:
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.24.0"]
+        NGINX_VERSION: ["1.24.0"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.25.5"]
+        NGINX_VERSION: ["1.25.5"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.26.3"]
+        NGINX_VERSION: ["1.26.3"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.27.5"]
+        NGINX_VERSION: ["1.27.5"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.28.2"]
+        NGINX_VERSION: ["1.28.2"]
+        WAF: ["OFF"]
+      - ARCH: ["amd64", "arm64"]
+        BASE_IMAGE: ["nginx:1.29.5"]
+        NGINX_VERSION: ["1.29.5"]
+        WAF: ["OFF"]
+
 coverage:
   extends:
     - .build-and-test-fast

.gitlab/common.yml

@@ -17,6 +17,19 @@
     paths:
       - artifacts/$ARCH/$NGINX_VERSION/$WAF/
 
+.build-nginx-rum:
+  extends: .build
+  variables:
+    RUM: "ON"
+  script:
+    - git config --global url."https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/".insteadOf "https://github.com/DataDog/"
+    - make build-musl-aux
+    - mkdir -p artifacts-rum/$ARCH/$NGINX_VERSION/$WAF
+    - cp .musl-build/ngx_http_datadog_module.so* artifacts-rum/$ARCH/$NGINX_VERSION/$WAF/
+  artifacts:
+    paths:
+      - artifacts-rum/$ARCH/$NGINX_VERSION/$WAF/
+
 .build-openresty:
   extends: .build
   script:
@@ -51,6 +64,16 @@
     paths:
       - test/logs/test.log
 
+.test-nginx-rum:
+  extends: .test
+  variables:
+    RUM: "ON"
+  script:
+    - python3 test/bin/run.py --image $BASE_IMAGE --module-path artifacts-rum/$ARCH/$NGINX_VERSION/$WAF/ngx_http_datadog_module.so -- --verbose --failfast
+  artifacts:
+    paths:
+      - test/logs/test.log
+
 .test-ingress-nginx:
   extends: .test
   variables:

CMakeLists.txt

@@ -105,16 +105,26 @@ if(NGINX_DATADOG_ASM_ENABLED)
 endif()
 
 if(NGINX_DATADOG_RUM_ENABLED)
-  execute_process (
-    COMMAND cargo pkgid --package inject-browser-sdk --manifest-path "${CMAKE_CURRENT_SOURCE_DIR}/deps/inject-browser-sdk/Cargo.toml"
-    OUTPUT_VARIABLE RUM_SDK_INJECTOR_PKGID
-  )
-  string(REGEX REPLACE ".*@(.*)" "\\1" RUM_SDK_INJECTOR_VERSION ${RUM_SDK_INJECTOR_PKGID})
-  string(STRIP "${RUM_SDK_INJECTOR_VERSION}" RUM_SDK_INJECTOR_VERSION)
-
-  unset(RUM_SDK_INJECTOR_PKGID)
-
   add_subdirectory(deps)
+
+  # Determine the inject-browser-sdk version for the build identifier
+  find_file(_inject_sdk_cargo_toml "Cargo.toml"
+    PATHS
+      "${CMAKE_CURRENT_SOURCE_DIR}/deps/inject-browser-sdk"
+      "${injectbrowsersdk_SOURCE_DIR}"
+    NO_DEFAULT_PATH)
+  if(_inject_sdk_cargo_toml)
+    execute_process(
+      COMMAND cargo pkgid --package inject-browser-sdk --manifest-path "${_inject_sdk_cargo_toml}"
+      OUTPUT_VARIABLE RUM_SDK_INJECTOR_PKGID
+    )
+    string(REGEX REPLACE ".*@(.*)" "\\1" RUM_SDK_INJECTOR_VERSION ${RUM_SDK_INJECTOR_PKGID})
+    string(STRIP "${RUM_SDK_INJECTOR_VERSION}" RUM_SDK_INJECTOR_VERSION)
+    unset(RUM_SDK_INJECTOR_PKGID)
+    unset(_inject_sdk_cargo_toml CACHE)
+  else()
+    set(RUM_SDK_INJECTOR_VERSION "unknown")
+  endif()
 endif()
 
 include(./cmake/generate_build_id.cmake)

Makefile

@@ -59,7 +59,7 @@ build-push-musl-toolchain:
 
 .PHONY: build-local-musl-toolchain
 build-local-musl-toolchain:
-	docker build --progress=plain --platform $(DOCKER_PLATFORM) --build-arg ARCH=$(ARCH) -t $(BUILD_IMAGE) build_env
+	docker build --progress=plain --platform $(DOCKER_PLATFORM) --build-arg ARCH=$(ARCH) $(if $(BASE_IMAGE), --build-arg BASE_IMAGE=$(BASE_IMAGE),) -t $(BUILD_IMAGE) build_env
 
 .PHONY: build-push-test-image
 build-push-test-image:

build_env/Dockerfile

@@ -1,6 +1,7 @@
 # Dockerfile for registry.ddbuild.io/ci/nginx-datadog/nginx_musl_toolchain
 
-FROM registry.ddbuild.io/images/mirror/library/alpine:3.20.3 AS sysroot
+ARG BASE_IMAGE=registry.ddbuild.io/images/mirror/library/alpine:3.20.3
+FROM ${BASE_IMAGE} AS sysroot
 
 ARG LLVM_VERSION=17.0.6
 ARG ARCH

deps/CMakeLists.txt

@@ -1,10 +1,15 @@
-include(corrosion.cmake)
+include(FetchContent)
 
-corrosion_import_crate(MANIFEST_PATH inject-browser-sdk/lib/Cargo.toml)
+set(INJECT_BROWSER_SDK_GIT_REF "49245e1c6ed8275990bb910125187e0aaad09e3d" CACHE STRING
+    "Git tag/branch for inject-browser-sdk")
 
-corrosion_experimental_cbindgen(
-  TARGET inject_browser_sdk
-  HEADER_NAME injectbrowsersdk.h
-  FLAGS --config ${CMAKE_SOURCE_DIR}/deps/cbindgen.toml
+FetchContent_Declare(
+  InjectBrowserSDK
+  GIT_REPOSITORY https://github.com/DataDog/inject-browser-sdk.git
+  GIT_TAG        ${INJECT_BROWSER_SDK_GIT_REF}
 )
+FetchContent_MakeAvailable(InjectBrowserSDK)
 
+if(NOT TARGET inject_browser_sdk)
+  add_library(inject_browser_sdk ALIAS inject_browser_sdk_ffi)
+endif()

deps/corrosion.cmake

@@ -113,6 +113,9 @@ ngx_int_t DatadogContext::on_header_filter(ngx_http_request_t *request) {
         rum_span.set_error(true);
       }
     } else {
+      // No trace/span found for this request (e.g. tracing is disabled via
+      // `datadog_tracing off`, or this is an untracked subrequest).
+      // Proceed with RUM injection without instrumentation.
       rum_ctx_.on_header_filter(request, loc_conf, ngx_http_next_header_filter);
     }
   }
@@ -340,6 +343,13 @@ void destroy_datadog_context(ngx_http_request_t *request) noexcept {
 }
 
 ngx_int_t DatadogContext::on_precontent_phase(ngx_http_request_t *request) {
+  // When tracing is disabled (e.g. `datadog_tracing off`), no traces are
+  // created. Skip header injection to avoid undefined behavior from
+  // calling traces_.front() on an empty vector.
+  if (traces_.empty()) {
+    return NGX_DECLINED;
+  }
+
   // inject headers in the precontent phase into the request headers
   // These headers will be copied by ngx_http_proxy_create_request on the
   // content phase into the outgoing request headers (probably)