Avoid warning when RUM is unconfigured

src/rum/config.cpp

@@ -109,6 +109,10 @@ std::optional<int> 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 <typename... Args>
 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<std::string_view> 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) {

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,

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';
+        }
+    }
+}

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';
+        }
+    }
+}

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()