Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions astro/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Prettier does not read .gitignore (it only auto-skips node_modules), so the
# generated and vendored trees have to be listed again here.
node_modules/
dist/
.astro/
.yarn/
test-results/
playwright-report/

# Playwright screenshot baselines. Nothing in them is formattable, but keeping
# them out means `format:check` never walks a large binary tree.
**/*-snapshots/
9 changes: 9 additions & 0 deletions astro/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ This site will eventually have tens of thousands of pages, with most of them ren
- If you recommend a different approach than what was asked for, query the user before proceeding.
- Test red to green: Write tests first, and verify that they are failing before implementing the code that will make them pass.

## Formatting

- After editing code, run `yarn format` from `astro/` — but scope it to the files you touched (`npx prettier --write <paths>`) unless the user has asked for a project-wide sweep. Most of the tree is not yet Prettier-clean, so a bare `yarn format` rewrites hundreds of unrelated files.
- Run it from `astro/`, never from the repo root. Prettier resolves the *nearest* config to each file and uses it wholesale (there is no cascade or `extends`), so files here get `astro/.prettierrc` — 2-space, plus `prettier-plugin-astro`. The repo-root `prettier.config.js` is Hugo's (4-space, single quotes, 120 cols) and must not be applied to this project.
- Use `yarn format:check` to see what would change without writing.
- `astro/.prettierignore` exists because Prettier does not read `.gitignore`. Add generated or vendored trees there rather than letting Prettier walk them.

## Testing

- When possible, test files should be localized with the code they're testing.
Expand Down Expand Up @@ -74,6 +81,8 @@ verification belongs here; anything that talks to a cloud provider does not.
- `yarn dev` — Start dev server on port 4321
- `yarn build` — Production build to `dist/`
- `yarn preview` — Preview the production build locally
- `yarn format` — Prettier-format this project in place (project-wide; see Formatting)
- `yarn format:check` — Report unformatted files without writing

## Stack

Expand Down
20 changes: 13 additions & 7 deletions astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@
"node": ">=24.0.0"
},
"scripts": {
"dev": "astro dev --port 4321",
"dev:proxied": "PROXIED=1 astro dev --port 4321",
"build": "astro build",
"build:en": "SKIP_TRANSLATIONS=true astro build",
"build:preview": "CI_ENVIRONMENT_NAME=preview astro build",
"build:live": "CI_ENVIRONMENT_NAME=live astro build",
"//build:ask-ai": "Called explicitly by every script that needs dist/, because Yarn Berry ignores arbitrary pre* hooks. TODO: assumes the Astro CI job invokes one of the build:* scripts rather than astro build directly — confirm when that job is written in DataDog/documentation-ci.",
"build:ask-ai": "yarn --cwd ../shared/packages/ask-ai build",
"dev": "yarn build:ask-ai && astro dev --port 4321",
"dev:proxied": "yarn build:ask-ai && PROXIED=1 astro dev --port 4321",
"build": "yarn build:ask-ai && astro build",
"build:en": "yarn build:ask-ai && SKIP_TRANSLATIONS=true astro build",
"build:preview": "yarn build:ask-ai && CI_ENVIRONMENT_NAME=preview astro build",
"build:live": "yarn build:ask-ai && CI_ENVIRONMENT_NAME=live astro build",
"verify:dist": "node scripts/verifyDist.mjs",
"typecheck": "astro check",
"//format": "Uses astro/.prettierrc, not the repo-root prettier.config.js — Prettier picks the nearest config and uses it wholesale, so never run this from the repo root or pass the root config.",
"format": "prettier --write .",
"format:check": "prettier --check .",
"typecheck": "yarn build:ask-ai && astro check",
"preview": "astro preview --port 4322",
"preview:proxied": "PROXIED=1 astro preview --port 4321",
"pretest": "node -e \"const [major] = process.versions.node.split('.').map(Number); if (major < 24) { console.error('Error: Node >=24 required (current: ' + process.version + '). Run: nvm use'); process.exit(1); }\"",
Expand All @@ -33,6 +38,7 @@
"@astrojs/sitemap": "^3.7.3",
"@datadog/browser-logs": "^7.11.0",
"@datadog/browser-rum": "^7.11.0",
"@dd/ask-ai": "link:../shared/packages/ask-ai",
"astro": "^7.1.5",
"cdocs-data": "https://s3.amazonaws.com/origin-static-assets/corp-node-packages/master/cdocs-data-v2.0.0.tgz",
"marked": "^18.0.7",
Expand Down
198 changes: 146 additions & 52 deletions astro/plans/22_add_rum.md

Large diffs are not rendered by default.

718 changes: 716 additions & 2 deletions astro/plans/23_ask_ai.md

Large diffs are not rendered by default.

239 changes: 236 additions & 3 deletions astro/plans/24_feature_flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,253 @@ rolled out to 100% of users, and the flag is a rarely-used kill switch. If it ev
needs to be pulled during that window, Astro requires a separate action.

Because that asymmetry is invisible from either site — nothing errors, nothing
warns, and the Datadog UI shows the flag as configured — plan 23 should leave a
warns, and the Datadog UI shows the flag as configured —
[23_ask_ai.md](23_ask_ai.md) should leave a
`TODO` at the Astro mount site where the resolver would be passed. Otherwise the
only record of it is this document, and the person reaching for the kill switch
during an incident is not reading plans:

```ts
// TODO: no flag resolver is passed here, so `docs-ai-enabled` does not govern
// Astro — the package falls back to its `true` default. Flipping the flag off
// disables Ask AI on Hugo only. Supply a resolver once plans/24_feature_flags.md
// lands, or if the kill switch is ever needed on Astro before then.
// disables Ask AI on Hugo only. Supply a resolver once Astro has a feature-flag
// client, or if the kill switch is ever needed on Astro before then.
```

## Claude's plan

### Confirmed decisions

| Question | Decision |
| --- | --- |
| Ownership | **The host owns flags**, not the Ask AI package. Option C from the design discussion, chosen so a second flag has somewhere to live that is not inside one component's package. |
| Library | **`@openfeature/web-sdk` + `@datadog/openfeature-browser`**, matching Hugo. One provider, one vendor. |
| Credentials | **Reuse `src/config/telemetry.ts`.** Its `applicationId` / `clientToken` are exactly what `DatadogProvider` needs, and they are already the same values Hugo's provider uses. No second table. |
| Initialization | **One memoized promise**, module-scoped, mirroring Hugo's `feature-flags.js`. Repeated callers share one provider and one exposure stream. |
| Targeting key | **RUM session ID**, falling back to `crypto.randomUUID()`. Same as Hugo. |
| Exposure logging | **On** (`enableExposureLogging: true`), matching Hugo. |
| Development env | **Mapped to `preview`**, as Hugo does. Development has no `applicationId`, so without the mapping flags cannot resolve locally at all. |
| Failure behavior | **Return `null`, log a warning, let every read fall back to its default.** A flag-service outage must not take Ask AI down. |
| Astro delivery | A bundled `<script>`-carrying component, ordered **after** `<Telemetry />` so the session ID exists when the targeting key is read. |
| Scope of this plan | **Astro only.** Hugo's `helpers/feature-flags.js` is untouched, including by [25_migrate_hugo_to_ask_ai_package.md](25_migrate_hugo_to_ask_ai_package.md). |

### Core idea

This is a small plan pretending to be a large one. The whole deliverable is a port of
Hugo's 56-line `feature-flags.js`, minus the ten lines of `fetchDatadogUserStatus()`
that [22_add_rum.md](22_add_rum.md) already duplicated into
`src/lib/telemetry/datadogUserStatus.ts`, plus one line at the Ask AI mount site.

What makes it worth a plan at all is not the code, it is two orderings and one absent
default:

- **RUM must initialize before the targeting key is read**, or every page load buckets
the visitor under a fresh random UUID and a percentage rollout becomes noise.
- **The provider must resolve before a read is trusted**, which the web SDK's
fetch-then-read-synchronously shape handles — but the Ask AI package mounts
optimistically *before* that resolution, so the flag arrives late by design.
- **Absent means enabled.** Every layer defaults permissive: no resolver means enabled,
a failed provider means enabled, an unknown flag means enabled. A kill switch that
fails closed would take the feature down on a flag-service outage.

Structurally it mirrors the telemetry work: one pure-ish module with the logic, one
thin component that touches `window`.

### Steps summary

| Step | What it does | Why it is separate |
| --- | --- | --- |
| 1. Dependencies | Adds the two OpenFeature packages. | Two new runtime deps on a site that ships very little JS; worth its own line in the diff. |
| 2. Flag resolution module | The memoized provider setup and the typed read helpers. | The only part with logic, and therefore the only part a unit test can reach. |
| 3. Flag keys | One module naming `docs-ai-enabled` and its default. | So the string exists once. Hugo exports it from the widget, which is backwards. |
| 4. Entry point | The component that kicks off initialization, ordered after `<Telemetry />`. | The ordering *is* the step. Everything else works and quietly buckets wrong. |
| 5. Supply the resolver to Ask AI | Pass `isEnabled` at the mount site and delete the `TODO`. | The only user-visible change in the plan. Everything above is inert without it. |

### 1. Dependencies

```
@openfeature/web-sdk
@datadog/openfeature-browser
```

Hugo pins `^1.7.2` and `^0.3.1`. Match those majors so both sites resolve the same flag
against the same provider version — a provider disagreement between hosts would be
invisible until the switch was thrown. Note the Datadog provider is pre-1.0, so treat
its minor bumps as breaking.

### 2. Flag resolution (`src/lib/flags/featureFlags.ts`)

```ts
export function initializeFeatureFlags(): Promise<Client | null>;
export function getBooleanFlag(client: Client | null, key: string, fallback: boolean): boolean;
```

A port of Hugo's, with the differences all being things TypeScript forces:

- The memoized `clientPromise` is module-scoped, so N callers produce one provider.
Same as Hugo, and the reason Hugo's two consumers do not double-log exposures.
- The env is resolved through `resolveSiteEnv(import.meta.env.PUBLIC_CI_ENV)` and then
**mapped `development` → `preview`**, reproducing Hugo's
`rawEnv === 'development' ? 'preview' : rawEnv`. Hugo's line carries a
"remove once rollout is complete" TODO; carry that forward rather than silently
keeping the mapping, but do keep it — development has no `applicationId`, so without
it local flag resolution is impossible rather than merely unrepresentative.
- Credentials come from `getTelemetryConfig(env)`. `applicationId` is optional on
`TelemetryCredentials`, so the guard Hugo writes as
`if (!config?.ddClientToken || !config?.ddApplicationId)` becomes a real narrowing
rather than a runtime check — which is the whole reason not to add a second
credentials table.
- Hugo's unused `getStringFlag` is not ported. Add it when something needs it.

`getBooleanFlag` keeps Hugo's shape — `client?.getBooleanValue(key, fallback) ?? fallback`
— so a null client from a failed init is indistinguishable from a resolved default at
the call site. That is intentional: the caller should not be able to tell, because both
mean "use the default".

Do **not** read `document.documentElement.dataset.env` the way Hugo does. Astro has
`data-env` on `<html>` ([22_add_rum.md](22_add_rum.md), section 7) and it would work,
but the build-time constant is the authority everywhere else in Astro's client code and two sources for
one value is how they drift.

### 3. Flag keys (`src/lib/flags/keys.ts`)

```ts
export const DOCS_AI_ENABLED = { key: "docs-ai-enabled", default: true } as const;
```

One place, with the default beside the key so they cannot disagree between call sites.
Hugo exports `DOCS_AI_ENABLED_FLAG_KEY` from the Ask AI widget itself and
`searchbarHits.js` imports it from there — which is why Hugo's searchbar depends on the
widget module. Astro does not repeat that.

### 4. Entry point (`src/components/FeatureFlags/FeatureFlags.astro`)

A bundled `<script>` carrier following `Telemetry.astro`'s shape, rendered by
`BaseLayout` **after** `<Telemetry />`.

The ordering is the entire content of this step. Astro emits bundled scripts as
deferred modules that execute in document order, and `datadogRum.init()` is synchronous
within Telemetry's script, so a later script sees a session ID. Rendering
`<FeatureFlags />` before `<Telemetry />` would still work — and would silently bucket
every visitor under a random UUID. Comment the ordering at the render site, not just
here, since the failure is invisible and the fix is a one-line reorder.

Two things follow from that ordering:

- **In development there is no session ID**, because
[22_add_rum.md](22_add_rum.md)'s env gate means RUM never
initializes locally. The targeting key falls back to `crypto.randomUUID()`, so local
flag evaluation is unbucketed. Same as Hugo, and acceptable, but it means percentage
rollouts cannot be tested locally at all — only on preview.
- **Soft navigation does not re-run it**, since bundled scripts do not re-execute under
`<ClientRouter />`. Correct: the provider should be set once per page load, and the
memoized promise would no-op anyway.

The script calls `initializeFeatureFlags()` and does nothing with the result. It exists
to start the fetch early; consumers await the same memoized promise themselves.

### 5. Supply the resolver to Ask AI

At Astro's Ask AI mount site (`src/components/AskAi/AskAi.astro`, from
[23_ask_ai.md](23_ask_ai.md)), replace the `TODO` quoted above with:

```ts
mountAskAi({
isEnabled: async () => {
const client = await initializeFeatureFlags();
return getBooleanFlag(client, DOCS_AI_ENABLED.key, DOCS_AI_ENABLED.default);
},
getIsDatadogUser: fetchDatadogUserStatus,
});
```

That is the whole integration. The package already implements the optimistic mount and
the teardown-on-false; this plan only supplies the boolean.

**Delete the `TODO` in the same commit.** It is the record of the asymmetry this plan
removes, and a stale one is worse than none — the next person reaching for the kill
switch during an incident would read it and conclude Astro is not covered when it is.

Astro's searchbar row (also from [23_ask_ai.md](23_ask_ai.md)) is a second consumer: it
should hide the "Ask AI about …" row when the flag is off, the way `searchbarHits.js` does. It reads
the same memoized promise, so this adds no second provider — the thing the host-owned
design exists to guarantee.

### Files

| File | Change |
| --- | --- |
| `astro/package.json` | Add `@openfeature/web-sdk`, `@datadog/openfeature-browser` |
| `astro/src/lib/flags/featureFlags.ts` | New — memoized provider setup and read helpers |
| `astro/src/lib/flags/keys.ts` | New — `docs-ai-enabled` and its default |
| `astro/src/components/FeatureFlags/FeatureFlags.astro` | New — starts initialization, ordered after `<Telemetry />` |
| `astro/src/layouts/BaseLayout.astro` | Render `<FeatureFlags />` after `<Telemetry />` |
| `astro/src/components/AskAi/AskAi.astro` | Pass `isEnabled`; delete the asymmetry `TODO` |
| `astro/src/components/SearchBar/SearchResultsPopup.tsx` | Hide the Ask AI row when the flag is off |

### Testing (red → green)

Unit (`vitest`), written first and verified failing. The provider is mocked throughout
— none of these should reach the network:

- `featureFlags.unit.test.ts` — `initializeFeatureFlags()` called three times sets one
provider (the assertion that the memoization holds, and therefore that exposures are
not duplicated); returns `null` and warns when `applicationId` is absent; returns
`null` rather than throwing when `setProviderAndWait` rejects; passes the RUM session
ID as `targetingKey` when `window.DD_RUM.getInternalContext()` supplies one, and a
UUID when it does not; resolves `development` to the `preview` credentials.
- `getBooleanFlag` — returns the client's value when there is one, the fallback for a
`null` client, and the fallback for an unknown key.

Browser (`playwright`), covering what unit tests cannot:

- The Ask AI widget is present with no flag configuration reachable — the
fail-open default, which is the behavior that matters during an outage and the one
most likely to regress silently.
- Zero console errors on a page where the provider cannot initialize. Development is
exactly that page, since flags resolve against preview credentials from a localhost
origin.

The flag actually being *off* is not automatable here — it needs a real flag change in
Datadog, which is verification section C.

### TODOs to leave in the code

| Location | TODO | Kind |
| --- | --- | --- |
| `src/lib/flags/featureFlags.ts`, the env mapping | `development` → `preview` is carried over from Hugo's own "remove once Feature Flag Rollout is complete" TODO. Reproduced deliberately; drop it when Hugo's goes. | Dies with Hugo |
| `src/lib/flags/featureFlags.ts` | Deliberate duplicate of Hugo's `helpers/feature-flags.js`, which stays in place for the searchbar. Cross-reference the twin so a change to one is not made blind to the other. | Dies with Hugo |
| `src/lib/flags/keys.ts` | `docs-ai-enabled` is the only flag, and it is a kill switch at 100% rollout. If it is retired without a replacement, this whole module goes with it — see verification G. | Permanent |

### Risks and open questions

- **This plan may not be worth implementing.** Stated in the prompt and still true: Ask
AI is at 100% rollout, the flag is a kill switch nobody has needed, and the fail-open
default means the only scenario this plan improves is one where someone deliberately
turns Ask AI off. Verification section G is the decision point, and "revert it" is a
legitimate outcome.
- **One switch now governs two sites.** The upside of sharing Hugo's RUM application.
Testing the flag on Astro's preview also disables Ask AI on Hugo's preview for
whoever is targeted — called out in verification C, and worth telling the flag's
owner before the first test rather than after.
- **Two OpenFeature registries on one origin** is the untested configuration. Hugo's
bundle and Astro's bundle each carry their own module instance, so a visitor moving
between the two sites within a session initializes two providers and produces two
exposure records. Not a correctness problem for a boolean, but it means exposure
counts on a shared application are not visitor counts. Nothing in this plan can fix
it while both sites exist.
- **The targeting-key ordering fails silently.** If `<FeatureFlags />` ever renders
before `<Telemetry />`, or if RUM's init becomes async, every visitor gets a fresh
UUID and bucketing breaks with no error. The unit test covers the fallback path but
cannot cover the ordering; verification B is the only real check.
- **Bundle weight, again.** Two more packages on top of
[22_add_rum.md](22_add_rum.md)'s two SDKs and [23_ask_ai.md](23_ask_ai.md)'s widget.
Deferred, so off the critical path, but the cumulative JS on a page that
used to ship almost none is now worth measuring once rather than assuming.
- **Exposure logging on a kill switch is cost without benefit.** It exists for
experiment analysis, and there is no experiment. It matches Hugo, which is the reason
to keep it, but if exposure volume ever matters it is the first thing to turn off.

## Manual verification

A kill switch that has never been tested is not a kill switch. The point of these
Expand Down
Loading
Loading