-
Notifications
You must be signed in to change notification settings - Fork 162
✨ [RUM-8780] Make developer extension capable of injecting the browser SDK #3877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 5e9f9ce | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
| } | ||
|
|
||
| return 'us1' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can probably remove this, the thing is that I was getting 403 so this was the fallback.
Gonna look for a better way.
(Open to suggestions)
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
|
@codex, can you review this pr please 🫶 |
There was a problem hiding this comment.
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.
ℹ️ 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".
| if (settings.injectionVariant === 'local-dev' && settings.useDevBundles === 'npm') { | ||
| injectDevBundle(settings.useRumSlim ? DEV_RUM_SLIM_URL : DEV_RUM_URL, ddRumGlobal) | ||
| injectDevBundle(DEV_LOGS_URL, ddLogsGlobal) | ||
| } else if (settings.injectionVariant === 'cdn') { | ||
| injectCdnBundle(settings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gate CDN injection on Datadog mode state
The content script triggers CDN injection whenever settings.injectionVariant === 'cdn'. Because injectionVariant is persisted in storage, a user can enable Datadog mode, pick the CDN variant, and then disable Datadog mode. The UI hides the variant control, but the stored value remains cdn, so this branch still runs and injects scripts even though Datadog mode is off. This means turning off Datadog mode does not actually disable the CDN injection feature and there is no way to revert without re‑enabling Datadog mode. Consider resetting injectionVariant when Datadog mode is disabled or guarding the CDN path with settings.datadogMode.
Useful? React with 👍 / 👎.
| if (site?.endsWith('datad0g.com')) { | ||
| return 'us3' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔨 warning: this seems incorrect
| injectionVariant: InjectionVariant | ||
| sdkInjectionType: SdkInjectionType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔨 warning: The settings should be redesigned to make it more explicit and ideally avoid forbidden states. Things that should be clarified at the settings level:
useDevBundles: npmwon't work ifinjectionVariantis notcdn.- How can I disable injection?
- Why injection variant CDN only available in
datadogMode? - What's the difference between
injectionVariantandsdkInjectionTypejust by looking at property names?
My suggestion: keep things simple. Always inject both RUM and Logs CDN bundles with a default config. If dev bundle override is enabled, those bundles will be overridden (including rum-slim override). If config override is enabled, the config will be overridden. No need to do anything at the injection level.
So in the end you have a single option, inject: boolean. No need to enforce datadogMode usage, people can use it it's fine.
| export const CDN_RUM_URL = `${CDN_BASE_URL}/${CDN_VERSION}/datadog-rum.js` | ||
| export const CDN_RUM_SLIM_URL = `${CDN_BASE_URL}/${CDN_VERSION}/datadog-rum-slim.js` | ||
| export const CDN_LOGS_URL = `${CDN_BASE_URL}/${CDN_VERSION}/datadog-logs.js` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like those constants aren't used
| function getRumBundleUrl(bundle: 'rum' | 'rum-slim', site?: string): string { | ||
| const region = getCdnRegion(site) | ||
| return `${CDN_BASE_URL}/${region}/${CDN_VERSION}/datadog-${bundle}.js` | ||
| } | ||
|
|
||
| function getLogsBundleUrl(site?: string) { | ||
| const region = getCdnRegion(site) | ||
| return `${CDN_BASE_URL}/${region}/${CDN_VERSION}/datadog-logs.js` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 suggestion: Just include us1 prod rum and logs. No need to worry about regions, staging, etc.
Motivation
The idea is to be able to inject the SDK on a page that does not have it. Injecting the local version and CDN version.
Changes
Load the extension dist folder, in chrome://extensions/. And on the SDK side go to developer-extension and
yarn dev.Disable the deployed Developer Extension to not have issues.
Added under Datadog Employee mode, the possibility to inject using CDN the SDK on pages that do not have it.
Test instructions
Screen.Recording.2025-09-26.at.16.37.02.mov
Checklist