Skip to content

feat(flags): measure per-user experiments by assignment reason and exposure - #716

Merged
mthines merged 3 commits into
mainfrom
feat/flag-experiment-measurement
Oct 21, 2026
Merged

mthines merged 3 commits into
mainfrom
feat/flag-experiment-measurement

Conversation

@dash0-dev

@dash0-dev dash0-dev Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Why

Per-user 50/50 splits already work (signed-in users are bucketed by Supabase user id), but the result could not be measured honestly: RUM carried the variant without saying how it was assigned, so a developer override read as treatment, and nothing recorded who actually reached a flagged surface.

What changed

  • Each flag's reason (static/split/override) flows from the server evaluation into the provider and onto every RUM signal as lorekit.feature_flag.reason.<key>.
  • RUM flag attributes are now replaced on re-sync; the SDK's append left both variants on every signal after an override flip.
  • useFeatureFlagExposure emits feature_flag.exposed (semconv flag attributes) once per mount from a resolver; InsightsView calls it.
  • insights-visual declares a 50/50 experiment, disabled (on first, so ramping never moves a user back to off).
  • docs/feature-flags.md § Running an experiment: declare, expose, launch/ramp, measure (exposed users per arm, sample-ratio check, outcomes with overrides excluded), plus the known page-load tagging gap.

How to verify

  • cd packages/feature-flags && npx vitest run and cd packages/web && npx vitest run src/lib/{dash0-rum,analytics/track}.spec.ts

Notes for reviewers

Nothing user-visible changes (the experiment stays off), so the public docs and llms.txt are untouched; docs/feature-flags.md and docs/otel.md carry the contributor-facing changes. Page-load signals (page view, early web vitals, browser errors) still miss flag tags because init() runs before flags are known — documented, left for a follow-up.

mthines and others added 2 commits October 21, 2026 16:20
- carry each flag's resolution reason (static/split/override) from the
  server evaluation into the provider and onto RUM signals as
  lorekit.feature_flag.reason.<key>
- replace, rather than append, flag signal attributes on re-sync so an
  override flip no longer leaves both arms on every later signal
- add feature_flag.exposed, emitted once per mount from a flag's resolver
  with the semantic-convention flag attributes; InsightsView emits it
- declare insights-visual as a disabled 50/50 experiment (on listed first
  so ramping never moves a user back to off)

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Running an experiment: declare arms dark with the treatment first, count
exposure in the resolver, launch and ramp, measure exposed users per arm
with a sample-ratio check, and outcomes with overrides excluded. Documents
the reason attribute, the replace-on-resync behaviour, the initial page-load
tagging gap, and corrects the span reason values to the lowercase ones the
hook emits.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

The dashboard preview for this PR — redeployed on each push that changes the web app.

Project Deployment Actions Updated
lorekit Ready Ready Preview Oct 21, 2026 4:40pm UTC

Preview always points at this PR's latest commit; Deployment is the immutable build of fe85c44. · Comment /web-preview to force a redeploy. · Workflow logs

@dash0-dev

dash0-dev Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

⚪ 2 non-blocking findings · 2 notes

Reason and exposure plumbing holds: the deferred event lands after identify and the RUM sync, every caller is updated, the PromQL labels match Dash0's normalisation, and no regeneration is needed. Four low-severity doc/test notes.

Finding Where
⚪ 1 Launching also breaks a client.spec.ts case
Fix: Add 'and delete the disabled-experiment case in client.spec.ts' to step 3 and to registry.ts's description.
docs/feature-flags.md:299
⚪ 2 Outcome and exposure series share no label
Fix: Show the division, renaming feature_flag_result_variant to feature_flag_insights_visual with label_replace on the exposure side.
docs/feature-flags.md:339
Notes (2) — posted inline
Finding Where
3 nitpick: Dash0Provider is an earlier sibling of the page's <Suspense> in the dashboard layout, so its identify already runs first; only this provider's own ancestor sync needs the deferral.
Confidence 93
packages/web/src/components/providers/FeatureFlagsProvider.tsx:143
4 nitpick: override is not a semconv well-known feature_flag.result.reason value (custom values are allowed), so this list overstates it; call it this package's custom value.
Confidence 95
packages/feature-flags/src/otel-attributes.ts:45
Review details
Gate Details
Description vs. code ✅ Matches the diff: reason flow, replace-on-resync, exposure hook, disabled 50/50 declaration, docs
Prior review feedback ✅ Earlier review comments are resolved.
Documentation ✅ feature-flags.md § Running an experiment and otel.md cover the new attributes, event and steps
Self-review signals ✅ No debug logs, leftover TODOs, or unreviewed stubs.
Code review ⚠️ 2 non-blocking findings — see inline comments.
Run Details
Mode full · 494 lines in delta · tier deep · depth checkout
Anomaly ⚠️ 2 prepare-time anomalies (reviewer identity unknown (no --reviewer-login / PR_REVIEWER_LOGIN; /user 401s here) — relation defaulted to cross, +1 more)
Checked 19 of 19 changed files read · 7 possible issues → 4 confirmed → 4 posted
Reach 12 changed exports · 17 dependent files · 3 checked · 14 not checked · 3 changed files imported by 28 files, not traced
FeatureFlagsProvider — 2 consumer files · 2 checked
syncFeatureFlagRumAttributes — 2 consumer files · 2 checked
useFeatureFlagVariant — 4 consumer files · 0 checked
withFlagVariants — 3 consumer files · 0 checked
featureFlagOtelHook — 2 consumer files · 0 checked
+7 more
packages/web/src/lib/analytics/track.ts — imported by 22 files
packages/feature-flags/src/registry.ts — imported by 4 files
packages/web/src/components/insights/InsightsView.tsx — imported by 2 files
Found produced 7 → posted inline 2 · notes 2 · cleared 2 · carried forward 0 · deferred 0 · below-bar 0
Severity ⚪ 2 low
Standards ran — 3 governing docs, 75 bullets, all 19 files; 0 findings (registry regenerated, arms already split, nothing user-visible)
CI still pending: Typecheck, Test & Lint (affected), Web — Playwright e2e (local build + local Supabase), Web — Storybook interaction + visual tests (changed), Web — Vercel preview (changed) / Deploy Vercel preview · 2 passed
Memories 127 indexed · 5 used (2 knowledge · 1 hotspot · 1 rule · 1 lesson)

Memories used

  • rule reviewer-comment-relevance::quality:pattern-divergence:AnalyticsEvent@packages/web/src/lib/analytics/track.ts — seeded a pattern-divergence candidate on the new AnalyticsEvent member; verifier contradicted it
  • knowledge knowledge::FlagKey@packages/feature-flags/src/generated/flags.generated.ts — re-verified at 0c5a434: flags.generated.ts holds keys + value types only; generator --check in sync
  • knowledge knowledge::AnalyticsEvent@packages/web/src/lib/analytics/track.ts — re-verified: docs/otel.md second RUM catalog updated alongside track.ts (no missing-update)
  • hotspot hotspot::packages/web/src/lib/analytics/track.ts — finder checklist line (missing-update class) — checked, not present
  • lesson otel::omit-the-attribute-rather-than-emit-a-placeholder-outside-the-semconv-enum — verifier check: unknown is a semconv well-known reason, so flagReasonValue's fallback is in-enum

Nothing to report — measurability (skipped), integrations (not activated), 0 files skipped.

pr-reviewer · commit 0c5a434 · full review · how these findings are produced · updated Oct 21, 2026 4:34pm UTC

Comment thread docs/feature-flags.md
Comment thread docs/feature-flags.md Outdated
Comment thread packages/feature-flags/src/otel-attributes.ts Outdated
Comment thread packages/web/src/components/providers/FeatureFlagsProvider.tsx Outdated
- derive the dark-experiment test from the registry, so launching one is
  still a single enabled: true edit
- show the per-arm rate as one PromQL division, relabelling the exposure
  side onto the outcome side's label
- say the exposure deferral exists for the provider's own RUM sync
- name override as a custom reason value, not a semconv one

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@mthines
mthines marked this pull request as ready for review October 21, 2026 17:38
@mthines
mthines merged commit f0584c8 into main Oct 21, 2026
19 checks passed
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.

1 participant