From 26e1b3c01aaaf730d33c9bd0204d65a8e206c069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 29 Sep 2026 16:33:18 +0200 Subject: [PATCH 1/5] fix(rum): avoid warning for absent optional config --- src/rum/config.cpp | 7 +++++++ test/cases/rum/test_injection.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 53545619..7442c1f2 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -109,6 +109,8 @@ std::optional parse_rum_version(std::string_view config_version) { namespace { constexpr std::size_t err_buf_size = 256; +// SDK error 10 means no matching stable config. +constexpr int no_stable_config_error = 10; template char* conf_err(ngx_conf_t* cf, const char* fmt, Args... args) { @@ -224,6 +226,11 @@ void try_build_snippet_from_stable_config( auto snippet = make_stable_config_snippet(nullptr); if (snippet == nullptr || snippet->error_code) { + const char* enabled = std::getenv("DD_RUM_ENABLED"); + if (snippet != nullptr && snippet->error_code == no_stable_config_error && + !loc_conf->rum_enable && (enabled == nullptr || enabled[0] == '\0')) { + return; + } ngx_log_error(NGX_LOG_WARN, cf->log, 0, "nginx-datadog: failed to create RUM snippet from " "stable config: %s", diff --git a/test/cases/rum/test_injection.py b/test/cases/rum/test_injection.py index 8cd4d944..83bea84c 100644 --- a/test/cases/rum/test_injection.py +++ b/test/cases/rum/test_injection.py @@ -245,6 +245,25 @@ class TestRUMInjection(case.TestCase): requires_rum = True + def test_unconfigured_rum_logs_only_when_enabled(self): + config = self._read_conf("rum_stable_config_only.conf") + status, lines = self.orch.nginx_test_config(config, + "rum_unconfigured.conf") + self.assertEqual(0, status, lines) + self.assertFalse( + any("failed to create RUM snippet" in line for line in lines), + lines) + + enabled = config.replace( + "datadog_tracing off;", + "datadog_tracing off;\n datadog_rum on;") + status, lines = self.orch.nginx_test_config( + enabled, "rum_enabled_unconfigured.conf") + self.assertEqual(0, status, lines) + self.assertTrue( + any("failed to create RUM snippet" in line for line in lines), + lines) + def _read_conf(self, conf_file): return (Path(__file__).parent / "conf" / conf_file).read_text() From 1259eadada438ae119ad9248e2621db20ff7f017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 29 Sep 2026 16:45:40 +0200 Subject: [PATCH 2/5] fix(rum): silence expected warning when disabled --- src/rum/config.cpp | 4 +++- test/cases/orchestration.py | 10 +++++++--- test/cases/rum/test_injection.py | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 7442c1f2..4720f822 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -228,7 +228,9 @@ void try_build_snippet_from_stable_config( if (snippet == nullptr || snippet->error_code) { const char* enabled = std::getenv("DD_RUM_ENABLED"); if (snippet != nullptr && snippet->error_code == no_stable_config_error && - !loc_conf->rum_enable && (enabled == nullptr || enabled[0] == '\0')) { + !loc_conf->rum_enable && + (enabled == nullptr || enabled[0] == '\0' || + parse_bool(enabled) == false)) { return; } ngx_log_error(NGX_LOG_WARN, cf->log, 0, diff --git a/test/cases/orchestration.py b/test/cases/orchestration.py index dd805991..674d4163 100644 --- a/test/cases/orchestration.py +++ b/test/cases/orchestration.py @@ -829,7 +829,7 @@ def sync_nginx_access_log(self): return log_lines log_lines.append(line) - def nginx_test_config(self, nginx_conf_text, file_name): + def nginx_test_config(self, nginx_conf_text, file_name, extra_env=None): """Test an nginx configuration. Write the specified `nginx_conf_text` to a file in the nginx @@ -856,8 +856,12 @@ def nginx_test_config(self, nginx_conf_text, file_name): """ # "-T" means "don't allocate a TTY". This is necessary to avoid the # error "the input device is not a TTY". - command = docker_compose_command("exec", "-T", "--", "nginx", - "/bin/sh") + env_args = [] + if extra_env is not None: + for key, value in extra_env.items(): + env_args.extend(("--env", f"{key}={value}")) + command = docker_compose_command("exec", "-T", *env_args, "--", + "nginx", "/bin/sh") result = subprocess.run( command, input=script, diff --git a/test/cases/rum/test_injection.py b/test/cases/rum/test_injection.py index 83bea84c..88a37bb4 100644 --- a/test/cases/rum/test_injection.py +++ b/test/cases/rum/test_injection.py @@ -254,6 +254,26 @@ def test_unconfigured_rum_logs_only_when_enabled(self): any("failed to create RUM snippet" in line for line in lines), lines) + for value in ("false", "0", "no", "off"): + status, lines = self.orch.nginx_test_config( + config, + f"rum_{value}_unconfigured.conf", + extra_env={"DD_RUM_ENABLED": value}) + self.assertEqual(0, status, lines) + self.assertFalse( + any("failed to create RUM snippet" in line for line in lines), + lines) + + for value in ("true", "invalid"): + status, lines = self.orch.nginx_test_config( + config, + f"rum_{value}_unconfigured.conf", + extra_env={"DD_RUM_ENABLED": value}) + self.assertEqual(0, status, lines) + self.assertTrue( + any("failed to create RUM snippet" in line for line in lines), + lines) + enabled = config.replace( "datadog_tracing off;", "datadog_tracing off;\n datadog_rum on;") From a5aa29ca8cd307e5c4515bd98e2b65a1f282b1d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 29 Sep 2026 17:15:01 +0200 Subject: [PATCH 3/5] refactor(rum): extract rum_requested helper --- src/rum/config.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 4720f822..39dc5f09 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -132,6 +132,17 @@ SnippetPtr make_stable_config_snippet(const char* overlay_json) { snippet_cleanup); } +// True when the config or DD_RUM_ENABLED asks for RUM. +// An unknown DD_RUM_ENABLED value counts as a request. +bool rum_requested(const datadog::nginx::datadog_loc_conf_t& loc_conf) { + if (loc_conf.rum_enable) { + return true; + } + const char* raw = std::getenv("DD_RUM_ENABLED"); + std::string_view env = raw != nullptr ? raw : ""; + return !env.empty() && parse_bool(env).value_or(true); +} + void apply_rum_config_tags(datadog::nginx::datadog_loc_conf_t* loc_conf, const rum_config_map& config) { loc_conf->rum_remote_config_tag = "remote_config_used:false"; @@ -226,11 +237,9 @@ void try_build_snippet_from_stable_config( auto snippet = make_stable_config_snippet(nullptr); if (snippet == nullptr || snippet->error_code) { - const char* enabled = std::getenv("DD_RUM_ENABLED"); + // No stable config and nobody asked for RUM: nothing to warn about. if (snippet != nullptr && snippet->error_code == no_stable_config_error && - !loc_conf->rum_enable && - (enabled == nullptr || enabled[0] == '\0' || - parse_bool(enabled) == false)) { + !rum_requested(*loc_conf)) { return; } ngx_log_error(NGX_LOG_WARN, cf->log, 0, From 972ad1750522cb7c5c068d612bb77175eb62a398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 29 Sep 2026 17:29:38 +0200 Subject: [PATCH 4/5] docs(rum): clarify stable config error 10 --- src/rum/config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 39dc5f09..1ae27448 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -109,7 +109,7 @@ std::optional parse_rum_version(std::string_view config_version) { namespace { constexpr std::size_t err_buf_size = 256; -// SDK error 10 means no matching stable config. +// SDK error 10: stable config has no RUM keys, not even DD_RUM_ENABLED. constexpr int no_stable_config_error = 10; template From 8489b18a9b4b54ec1480c892b6561de2c0581e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 29 Sep 2026 19:15:35 +0200 Subject: [PATCH 5/5] fix(rum): address review feedback - Let the datadog_rum directive win over DD_RUM_ENABLED when deciding whether to warn about missing stable config. - Rename the helper to is_rum_requested and share DD_RUM_ENABLED lookup. - Point to where SDK error 10 is defined. - Add type hints, factor the test, and use conf files instead of string replacement. --- src/rum/config.cpp | 43 +++++++++----- test/cases/orchestration.py | 6 +- .../cases/rum/conf/rum_stable_config_off.conf | 34 +++++++++++ test/cases/rum/conf/rum_stable_config_on.conf | 34 +++++++++++ test/cases/rum/test_injection.py | 59 ++++++++----------- 5 files changed, 127 insertions(+), 49 deletions(-) create mode 100644 test/cases/rum/conf/rum_stable_config_off.conf create mode 100644 test/cases/rum/conf/rum_stable_config_on.conf diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 1ae27448..9e4f7c90 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -110,6 +110,8 @@ namespace { constexpr std::size_t err_buf_size = 256; // SDK error 10: stable config has no RUM keys, not even DD_RUM_ENABLED. +// The SDK is internal. Its error codes are listed on `Snippet` in +// deps/inject-browser-sdk/lib/inject-browser-sdk-ffi/src/snippet.rs. constexpr int no_stable_config_error = 10; template @@ -132,15 +134,25 @@ SnippetPtr make_stable_config_snippet(const char* overlay_json) { snippet_cleanup); } -// True when the config or DD_RUM_ENABLED asks for RUM. +// DD_RUM_ENABLED, or nullopt when unset or empty. +std::optional get_rum_enabled_env() { + const char* raw = std::getenv("DD_RUM_ENABLED"); + if (raw == nullptr || raw[0] == '\0') { + return std::nullopt; + } + return raw; +} + +// True when RUM is asked for. A `datadog_rum` directive wins. +// Without one, DD_RUM_ENABLED decides. // An unknown DD_RUM_ENABLED value counts as a request. -bool rum_requested(const datadog::nginx::datadog_loc_conf_t& loc_conf) { - if (loc_conf.rum_enable) { - return true; +bool is_rum_requested(const datadog::nginx::datadog_loc_conf_t& loc_conf, + bool rum_enable_unset) { + if (!rum_enable_unset) { + return loc_conf.rum_enable; } - const char* raw = std::getenv("DD_RUM_ENABLED"); - std::string_view env = raw != nullptr ? raw : ""; - return !env.empty() && parse_bool(env).value_or(true); + auto env = get_rum_enabled_env(); + return env.has_value() && parse_bool(*env).value_or(true); } void apply_rum_config_tags(datadog::nginx::datadog_loc_conf_t* loc_conf, @@ -232,14 +244,15 @@ char* on_datadog_rum_config(ngx_conf_t* cf, ngx_command_t* command, } void try_build_snippet_from_stable_config( - ngx_conf_t* cf, datadog::nginx::datadog_loc_conf_t* loc_conf) { + ngx_conf_t* cf, datadog::nginx::datadog_loc_conf_t* loc_conf, + bool rum_enable_unset) { try { auto snippet = make_stable_config_snippet(nullptr); if (snippet == nullptr || snippet->error_code) { // No stable config and nobody asked for RUM: nothing to warn about. if (snippet != nullptr && snippet->error_code == no_stable_config_error && - !rum_requested(*loc_conf)) { + !is_rum_requested(*loc_conf, rum_enable_unset)) { return; } ngx_log_error(NGX_LOG_WARN, cf->log, 0, @@ -263,8 +276,8 @@ void try_build_snippet_from_stable_config( void resolve_rum_enable_from_env(ngx_conf_t* cf, datadog::nginx::datadog_loc_conf_t* loc_conf) { - const char* raw = std::getenv("DD_RUM_ENABLED"); - if (raw == nullptr || raw[0] == '\0') { + auto raw = get_rum_enabled_env(); + if (!raw.has_value()) { // Auto-enable when a snippet is available (from a directive, parent // inheritance, or stable config) so users don't have to set // DD_RUM_ENABLED explicitly alongside their RUM configuration. @@ -274,12 +287,12 @@ void resolve_rum_enable_from_env(ngx_conf_t* cf, return; } - auto parsed = parse_bool(raw); + auto parsed = parse_bool(*raw); if (!parsed.has_value()) { ngx_log_error(NGX_LOG_WARN, cf->log, 0, - "nginx-datadog: unrecognized DD_RUM_ENABLED value '%s'; " + "nginx-datadog: unrecognized DD_RUM_ENABLED value '%*s'; " "expected true/false/1/0/yes/no/on/off", - raw); + raw->size(), raw->data()); return; } @@ -314,7 +327,7 @@ char* datadog_rum_merge_loc_config(ngx_conf_t* cf, } if (child->rum_snippet == nullptr) { - try_build_snippet_from_stable_config(cf, child); + try_build_snippet_from_stable_config(cf, child, rum_enable_unset); } if (rum_enable_unset) { diff --git a/test/cases/orchestration.py b/test/cases/orchestration.py index 674d4163..fa209b97 100644 --- a/test/cases/orchestration.py +++ b/test/cases/orchestration.py @@ -829,7 +829,11 @@ def sync_nginx_access_log(self): return log_lines log_lines.append(line) - def nginx_test_config(self, nginx_conf_text, file_name, extra_env=None): + def nginx_test_config( + self, + nginx_conf_text: str, + file_name: str, + extra_env: dict[str, str] | None = None) -> tuple[int, list[str]]: """Test an nginx configuration. Write the specified `nginx_conf_text` to a file in the nginx diff --git a/test/cases/rum/conf/rum_stable_config_off.conf b/test/cases/rum/conf/rum_stable_config_off.conf new file mode 100644 index 00000000..b0ce808b --- /dev/null +++ b/test/cases/rum/conf/rum_stable_config_off.conf @@ -0,0 +1,34 @@ +load_module /datadog-tests/ngx_http_datadog_module.so; + +error_log /dev/stdout debug; + +events { + worker_connections 1024; +} + +http { + server { + datadog_tracing off; + datadog_rum off; + + access_log /dev/stdout; + error_log /dev/stdout debug; + + root /datadog-tests/html; + listen 80; + server_name localhost; + + location / { + index index.html index.htm; + } + + location /disable-rum { + datadog_rum off; + try_files /index.html =404; + } + + location /healthcheck { + return 200 'ok'; + } + } +} diff --git a/test/cases/rum/conf/rum_stable_config_on.conf b/test/cases/rum/conf/rum_stable_config_on.conf new file mode 100644 index 00000000..e5801ac3 --- /dev/null +++ b/test/cases/rum/conf/rum_stable_config_on.conf @@ -0,0 +1,34 @@ +load_module /datadog-tests/ngx_http_datadog_module.so; + +error_log /dev/stdout debug; + +events { + worker_connections 1024; +} + +http { + server { + datadog_tracing off; + datadog_rum on; + + access_log /dev/stdout; + error_log /dev/stdout debug; + + root /datadog-tests/html; + listen 80; + server_name localhost; + + location / { + index index.html index.htm; + } + + location /disable-rum { + datadog_rum off; + try_files /index.html =404; + } + + location /healthcheck { + return 200 'ok'; + } + } +} diff --git a/test/cases/rum/test_injection.py b/test/cases/rum/test_injection.py index 88a37bb4..32c20533 100644 --- a/test/cases/rum/test_injection.py +++ b/test/cases/rum/test_injection.py @@ -247,42 +247,35 @@ class TestRUMInjection(case.TestCase): def test_unconfigured_rum_logs_only_when_enabled(self): config = self._read_conf("rum_stable_config_only.conf") - status, lines = self.orch.nginx_test_config(config, - "rum_unconfigured.conf") - self.assertEqual(0, status, lines) - self.assertFalse( - any("failed to create RUM snippet" in line for line in lines), - lines) - + self.assertFalse(self._is_snippet_failure_logged(config)) for value in ("false", "0", "no", "off"): - status, lines = self.orch.nginx_test_config( - config, - f"rum_{value}_unconfigured.conf", - extra_env={"DD_RUM_ENABLED": value}) - self.assertEqual(0, status, lines) - self.assertFalse( - any("failed to create RUM snippet" in line for line in lines), - lines) - + with self.subTest(DD_RUM_ENABLED=value): + self.assertFalse( + self._is_snippet_failure_logged(config, + {"DD_RUM_ENABLED": value})) for value in ("true", "invalid"): - status, lines = self.orch.nginx_test_config( - config, - f"rum_{value}_unconfigured.conf", - extra_env={"DD_RUM_ENABLED": value}) - self.assertEqual(0, status, lines) - self.assertTrue( - any("failed to create RUM snippet" in line for line in lines), - lines) - - enabled = config.replace( - "datadog_tracing off;", - "datadog_tracing off;\n datadog_rum on;") - status, lines = self.orch.nginx_test_config( - enabled, "rum_enabled_unconfigured.conf") + with self.subTest(DD_RUM_ENABLED=value): + self.assertTrue( + self._is_snippet_failure_logged(config, + {"DD_RUM_ENABLED": value})) + + # The `datadog_rum` directive wins over DD_RUM_ENABLED. + enabled = self._read_conf("rum_stable_config_on.conf") + self.assertTrue(self._is_snippet_failure_logged(enabled)) + disabled = self._read_conf("rum_stable_config_off.conf") + self.assertFalse( + self._is_snippet_failure_logged(disabled, + {"DD_RUM_ENABLED": "true"})) + + def _is_snippet_failure_logged( + self, + config: str, + extra_env: dict[str, str] | None = None) -> bool: + status, lines = self.orch.nginx_test_config(config, + "rum_unconfigured.conf", + extra_env=extra_env) self.assertEqual(0, status, lines) - self.assertTrue( - any("failed to create RUM snippet" in line for line in lines), - lines) + return any("failed to create RUM snippet" in line for line in lines) def _read_conf(self, conf_file): return (Path(__file__).parent / "conf" / conf_file).read_text()