✨ Feature Flags tab — all Datadog sites, with overrides scoped per site - #4965
Merged
Conversation
Expands FLAG_SITES beyond US1 + Staging now that the prod OAuth client has replicated to the other commercial DCs (US3, US5, EU1, AP1, AP2). FED/GovCloud stays excluded since the client isn't registered there.
…ear all Overrides live in the inspected page's localStorage, which has no Datadog site scoping, so one created on staging keeps applying on US1. The wrapper can't be changed, so detect it instead: look each overridden key up in the connected site's catalog and mark the row red when no active flag holds the key, or one does but the stored type disagrees. Only a well-formed lookup that matched nothing counts as evidence — a failed request, a still-loading one, or a 2xx whose body isn't the expected envelope must not tell the user to clear a working override. Also adds a Clear all to the signed-out notice (confirm step, error surfacing, and a reload prompt, since the page keeps applying overrides until it rebuilds its provider), and drops a getFlagsApiHost test that passed by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BeltranBulbarellaDD
left a comment
Contributor
There was a problem hiding this comment.
Small nits. But let's see if it would be nicer to just filter by site in the local storage. It's not the full solution but I think would simplify a little the code here. So we fetch only the ones on the logged in site for example.
All the rest looks good! I'll take a closer look after the changes.
Overrides live in the inspected page's localStorage under a single key the wrapper reads on init, with no site in it. localStorage is scoped per browser origin, not per Datadog site, so an override set on staging kept applying — invisibly — after switching to US1. We can't change the wrapper. So the extension keeps its own per-site stores and projects the connected site's into the key the wrapper reads. Staging's copy is parked rather than sitting where the wrapper can see it, which fixes the case detection never could: the same key existing in both sites with the same type. Projection is a no-op when nothing would change, so signing in stays inert and a reload is only ever demanded when the page really is running another site's values. Pre-scoping overrides are adopted once, gated on no site store holding anything, so no site inherits another's. A failed projection is surfaced rather than assumed to have worked. The cross-environment warning goes with it — an override in a site's store is that site's by construction. The row still notes a flag archived or deleted since the override was set, and still flags a type mismatch the wrapper would reject, now split by severity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bundles Sizes Evolution
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 8bdf3fb | Docs | View more details | Give us feedback! |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…omments The sync path had grown its own JSON parsing while the read and write paths used another, which is what the shared prelude existed to prevent. Both now use one `parse`/`stable` helper block. Comments trimmed to what isn't already obvious from the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A site switch repoints the projection before the reload that picks it up, so the page can still be applying the site it loaded with. The signed-out notice read only the projection, so after switching without reloading it showed nothing — no warning and no Clear all — while an override was still in effect. It now reports every stored override, which is also what Clear all wipes there. Copy follows: stored for this page, may still be applying. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BeltranBulbarellaDD
left a comment
Contributor
There was a problem hiding this comment.
Works great!
The tab shipped days ago, so there's no override history worth carrying forward. Overrides left in the wrapper key by an older build are now simply dropped on first connect instead of being adopted into the first site. That removes the adoption gate and, with it, the reason syncSiteOverrides returned the store at all — so the hook's sequence guard and its extra state write go too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Reload to apply US1's overrides" beats "this site's" — and with the site named in the title, the body no longer needs a word for the concept, so it drops the site/environment mix-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… crash the tab sanitizeOverrides deliberately keeps a shaped-but-malformed override so it stays visible and removable, so a stored `type` can be a non-string. The new type mismatch row renders that type, and returning it raw made React throw. A type named `toString` also found an inherited member, so `label` is what's checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the remaining Datadog sites to the Flags tab, and fixes a bug that expansion exposed.
The bug
The wrapper stores flag overrides in the inspected page's
localStorageunder a single key with no Datadog site in it.localStorageis scoped per browser origin, not per Datadog site — so staging and US1 share one bucket on the same page.Set an override on staging → reconnect to US1 → it's still there and still applying. If the same flag key exists in both environments with the same type (normal for a flag promoted through environments), nothing looks wrong at all: the row renders as an ordinary US1 override, and the page quietly behaves according to a value you set in staging.
The fix
The wrapper can't be changed, so the extension keeps its own per-site stores and projects the connected site's into the key the wrapper reads:
Staging's copy is parked where the wrapper can't see it. Switch to US1, reload, and the page uses US1's real values; switch back, reload, and your staging override returns. Nothing is deleted, and the wrapper needs no change.
Three properties worth calling out, because they're where the design decisions live:
Signing in stays inert. The projection is compared before it's written and skipped when nothing would change. Reconnect to the site you used last time and nothing is written and nothing is prompted — a reload is only ever demanded when the page genuinely is running another site's values.
Pre-scoping overrides are dropped, not migrated. The site store is the only source, so anything left in the wrapper's key by an older build is cleared on first connect. The tab shipped days ago, so there's no override history worth carrying forward — and skipping the migration removes the adoption gate along with everything guarding it.
A failed projection is surfaced, not assumed. If the write fails, the page may still be applying another site's overrides while the panel shows this site's. That now raises an error instead of passing silently.
Also in this PR
Site picker — adds US3, US5, EU1, AP1, AP2 (previously US1 + staging only). FED/GovCloud stays out: separate deployment, client isn't registered there.
Clear all before signing in. The signed-out notice warned that overrides were active but gave no way to act on them. It now has its own Clear all with a confirm step, surfaced failures, and a reload prompt (the page keeps applying them until it rebuilds its provider).
Signed out there's no site to scope to, so the notice reports every stored override and Clear all wipes all of them. Reporting only the wrapper's key would hide an override the page is still applying: a switch repoints that key before the reload that picks it up, so switching and then disconnecting without reloading would leave the notice empty while the old site's override was still in effect. Clearing only that key has the mirror problem — the rest would come back on reconnect, as if the button hadn't worked. The confirmation says it covers every site.
Row warnings, split by severity. The cross-environment warning is gone — an override in a site's store is that site's by construction. What remains:
Known limits
syncSiteOverrides.Testing
yarn typecheck,yarn lint,yarn formatcleanlocalStorage, so they exercise the projection and clear paths rather than mocks.Verified manually, both directions. Worth running both if you're reviewing this — the first shows the leak is closed, the second shows nothing was destroyed closing it. Use a flag with a visible effect (dark mode works well) that exists in both environments.
Leak is closed:
Nothing was destroyed:
Step 6 is the one that matters. Steps 1–4 alone can't tell a parked override from a deleted one; only the round trip shows the store survived the switch.