diff --git a/src/rum/config.cpp b/src/rum/config.cpp index 53545619..9e4f7c90 100644 --- a/src/rum/config.cpp +++ b/src/rum/config.cpp @@ -109,6 +109,10 @@ std::optional parse_rum_version(std::string_view config_version) { 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 char* conf_err(ngx_conf_t* cf, const char* fmt, Args... args) { @@ -130,6 +134,27 @@ SnippetPtr make_stable_config_snippet(const char* overlay_json) { snippet_cleanup); } +// 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 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; + } + 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, const rum_config_map& config) { loc_conf->rum_remote_config_tag = "remote_config_used:false"; @@ -219,11 +244,17 @@ 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 && + !is_rum_requested(*loc_conf, rum_enable_unset)) { + return; + } ngx_log_error(NGX_LOG_WARN, cf->log, 0, "nginx-datadog: failed to create RUM snippet from " "stable config: %s", @@ -245,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. @@ -256,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; } @@ -296,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 dd805991..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): + 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 @@ -856,8 +860,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/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 8cd4d944..32c20533 100644 --- a/test/cases/rum/test_injection.py +++ b/test/cases/rum/test_injection.py @@ -245,6 +245,38 @@ 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") + self.assertFalse(self._is_snippet_failure_logged(config)) + for value in ("false", "0", "no", "off"): + with self.subTest(DD_RUM_ENABLED=value): + self.assertFalse( + self._is_snippet_failure_logged(config, + {"DD_RUM_ENABLED": value})) + for value in ("true", "invalid"): + 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) + 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()