Avoid warning when RUM is unconfigured - #449
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91c27ff7c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
c3237aa to
1259ead
Compare
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 8489b18 | Docs | View more details | Give us 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.
Summary
DD_RUM_ENABLEDis unset or explicitly false and RUM is not enabled by an Nginx directive.is_rum_requestedhelper. Adatadog_rumdirective wins overDD_RUM_ENABLED, sodatadog_rum offstays quiet.datadog_rum onduring Nginx configuration checks.This PR is based directly on
masterand can be merged independently of #430.Validation
clang-format-14 --dry-run --Werror src/rum/config.cppyapf --diff test/cases/orchestration.py test/cases/rum/test_injection.pypython3 -m py_compile test/cases/orchestration.py test/cases/rum/test_injection.pygit diff --check origin/master...HEADDOCKER_CONTEXT=orbstack NGINX_VERSION=1.31.6 ARCH=aarch64 RUM=ON WAF=OFF TEST_ARGS=cases.rum.test_injection.TestRUMInjection.test_unconfigured_rum_logs_only_when_enabled make build-and-test(passed after rebase)a5aa29c):DOCKER_CONTEXT=orbstack NGINX_VERSION=1.28.3 ARCH=aarch64 RUM=ON WAF=OFF BASE_IMAGE=nginx:1.28.3 TEST_ARGS=cases.rum.test_injection make build-musl test(17 tests passed; re-run after review changes, 17 passed)DD_RUM_ENABLED: truestill warns (missingapplicationId), and one with onlyDD_RUM_ENABLED: falsestays quiet.