Skip to content

Avoid warning when RUM is unconfigured - #449

Merged
pawelchcki merged 5 commits into
masterfrom
pawel/quiet-unconfigured-rum
Sep 30, 2026
Merged

pawelchcki merged 5 commits into
masterfrom
pawel/quiet-unconfigured-rum

Conversation

@pawelchcki

@pawelchcki pawelchcki commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Skip the expected "no stable config" warning when DD_RUM_ENABLED is unset or explicitly false and RUM is not enabled by an Nginx directive.
  • Keep warnings for malformed configuration and for RUM enabled without a valid snippet.
  • Keep the "was RUM requested?" check in a small is_rum_requested helper. A datadog_rum directive wins over DD_RUM_ENABLED, so datadog_rum off stays quiet.
  • Test the unset value, all supported false values, true and malformed values, and explicit datadog_rum on during Nginx configuration checks.

This PR is based directly on master and can be merged independently of #430.

Validation

  • clang-format-14 --dry-run --Werror src/rum/config.cpp
  • yapf --diff test/cases/orchestration.py test/cases/rum/test_injection.py
  • python3 -m py_compile test/cases/orchestration.py test/cases/rum/test_injection.py
  • git diff --check origin/master...HEAD
  • DOCKER_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)
  • After extracting the helper (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)
  • Also checked by hand that a stable config with only DD_RUM_ENABLED: true still warns (missing applicationId), and one with only DD_RUM_ENABLED: false stays quiet.

@pawelchcki
pawelchcki requested a review from a team as a code owner September 29, 2026 14:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T14:36:04.337381Z 91c27ff PR opened
🔒 Security Review ✅ Completed 2026-09-29T14:36:21.718352Z 91c27ff PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/rum/config.cpp Outdated
@pawelchcki
pawelchcki requested a review from a team as a code owner September 29, 2026 14:45
@pawelchcki
pawelchcki force-pushed the pawel/quiet-unconfigured-rum branch from c3237aa to 1259ead Compare September 29, 2026 15:02
@pawelchcki
pawelchcki changed the base branch from validate_tracing to master September 29, 2026 15:02
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 69.29% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8489b18 | Docs | View more details | Give us feedback!

Comment thread src/rum/config.cpp
Comment thread src/rum/config.cpp Outdated
Comment thread src/rum/config.cpp Outdated
Comment thread src/rum/config.cpp
Comment thread test/cases/orchestration.py Outdated
Comment thread test/cases/rum/test_injection.py Outdated
Comment thread test/cases/rum/test_injection.py Outdated
- 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.
@pawelchcki
pawelchcki merged commit e7708d0 into master Sep 30, 2026
214 of 215 checks passed
@pawelchcki
pawelchcki deleted the pawel/quiet-unconfigured-rum branch September 30, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants