feat(experimentation): environment-scoped metrics & experiment results#7674
feat(experimentation): environment-scoped metrics & experiment results#7674gagantrivedi wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
Bot
added
api
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7674 +/- ##
==========================================
+ Coverage 98.52% 98.53% +0.01%
==========================================
Files 1444 1451 +7
Lines 55083 55469 +386
==========================================
+ Hits 54273 54659 +386
Misses 810 810 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
gagantrivedi
force-pushed
the
feat/experiment-metrics
branch
from
86ccf2a to
435d91f
Compare
github-actions
Bot
added
feature
gagantrivedi
force-pushed
the
feat/experiment-metrics
branch
from
435d91f to
9568226
Compare
github-actions
Bot
added
feature
gagantrivedi
force-pushed
the
feat/experiment-metrics
branch
from
9568226 to
2fea3fa
Compare
github-actions
Bot
added
feature
… attachment
Add a reusable, environment-scoped Metric and the ExperimentMetric join that
attaches metrics to experiments.
- Models: Metric (numeric; count/sum/mean/occurrence aggregations + JSON
definition), ExperimentMetric (expected_direction; one attach per
experiment+metric); Experiment gains exposure_event ($flag_exposure) and
control_variant.
- Metric library CRUD under environments/{key}/experiment-metrics/, gated on
EXPERIMENT_FLAG + environment admin. Metrics are immutable for now (no
update); deletion blocked while attached to an active experiment.
- Attach/detach metrics under an experiment, with same-environment and
unique-attach validation.
Results computation (ClickHouse query builder + statistics) is intentionally
kept on a separate branch; this branch is models + API only.
gagantrivedi
force-pushed
the
feat/experiment-metrics
branch
from
2fea3fa to
c961486
Compare
github-actions
Bot
added
feature
| expected_direction = models.CharField( | ||
| max_length=20, | ||
| choices=ExpectedDirection.choices, | ||
| ) |
There was a problem hiding this comment.
Interesting. So you see it in the relation Experiment x Metrics. I could see it too there although i'm wondering if that's a reality.
Let's say we have those metrics:
- Conversion rate (up)
- Average basket (up)
- Time to activation (down)
- First time page render (down)
Is there a world in which we'd want an experiment to push it in the other direction ? If not i'd stick it to the metrics and maybe have the possibility to override it in an experiment (in v2)
There was a problem hiding this comment.
Ok I think i'm actually mixing 2 things:
- the metric polarity (is it better up or is it better down as per what it is) -> I think we should also add this one
- the experiment impact (it should go up, it should keep it same level, it should impact it down) especially as a guardrail =>
expected_directionthat we should keep
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Metric and ExperimentMetric models, along with corresponding API endpoints, serializers, permissions, audit logging, and unit tests to support experiment metrics. The review feedback highlights several critical improvements for robustness and business logic validation: using get_object_or_404 to handle missing or soft-deleted experiments cleanly, excluding soft-deleted experiments when checking for active metric attachments, adding defensive validation in ExperimentMetricSerializer (such as preventing modifications to completed experiments or attaching deleted metrics), and restricting the detachment of metrics from completed experiments.
There was a problem hiding this comment.
Pull request overview
Adds a first-pass “metric library” for experimentation by introducing environment-scoped Metric objects and an ExperimentMetric join model, then exposing CRUD / attach / detach APIs under environment + experiment routes with auditing and permissions.
Changes:
- Add
Metric+ExperimentMetricmodels (with migration) and extend audit related-object types. - Introduce metric library endpoints (
/experiment-metrics/) and experiment metric attachment endpoints (/experiments/{id}/metrics/) with permissions, serializers, and audit logs. - Add unit tests covering metric CRUD, immutability expectations, attach/detach flows, and basic validation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| api/tests/unit/experimentation/test_metric_views.py | Adds API tests for environment-scoped metric library CRUD and permission / flag gating. |
| api/tests/unit/experimentation/test_metric_models.py | Adds model-level tests for defaults, uniqueness, and soft-delete behavior. |
| api/tests/unit/experimentation/test_experiment_metric_views.py | Adds API tests for attaching, listing, detaching, and updating experiment-metric relationships. |
| api/experimentation/views.py | Adds MetricViewSet and ExperimentMetricViewSet and wires metric audit logging + delete-guard logic. |
| api/experimentation/services.py | Adds create_metric_audit_log and reuses existing feature-flag helpers. |
| api/experimentation/serializers.py | Adds MetricSerializer and ExperimentMetricSerializer with definition + attachment validation. |
| api/experimentation/permissions.py | Adds MetricPermission to gate metric library endpoints on experiment flag + env admin. |
| api/experimentation/models.py | Introduces MetricAggregation, ExpectedDirection, Metric, and ExperimentMetric. |
| api/experimentation/migrations/0005_metrics.py | Creates DB tables for Metric and ExperimentMetric. |
| api/experimentation/metric_urls.py | Registers the metric library router under the environment. |
| api/experimentation/experiment_urls.py | Adds nested routes for /experiments/{id}/metrics/ using nested routers. |
| api/environments/urls.py | Includes the new experiment-metrics URL module under environments. |
| api/audit/related_object_type.py | Adds METRIC related object type for audit logs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
added
feature
for more information, see https://pre-commit.ci
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Metric and ExperimentMetric models, along with their corresponding serializers, viewsets, nested API routing, and audit logging, to support associating versioned metrics with experiments. Comprehensive unit tests and database migrations are also included. The review feedback suggests two key improvements: adding a guard check in ExperimentMetricSerializer.validate to prevent a potential AttributeError when metric is None during partial updates, and caching the retrieved Experiment instance in ExperimentMetricViewSet._get_experiment to avoid redundant database queries.
github-actions
Bot
added
feature
| METRIC_DEFINITION_VALIDATORS: dict[int, DefinitionValidator] = { | ||
| 1: _validate_v1, | ||
| } |
There was a problem hiding this comment.
The way I see it to evolve the schema without breaking existing experiments is to add / create a new shape and version it here.
I can see the stat engine also use versioning to generate the queries. Wdyt ?
There was a problem hiding this comment.
Yeah, that was the idea
github-actions
Bot
added
feature
github-actions
Bot
added
feature
What
Adds a reusable, environment-scoped
Metricand wires metrics into experiments end to end, ClickHouse-native. Builds on the existingexperimentationapp (Experiment,WarehouseConnection).Data model
Metric— environment-scoped, soft-delete.metric_type(numeric/conversion),aggregation(count/sum/mean), and a JSONdefinition(the recipe: event + optional filters/value/window). Immutable for now (no update endpoint).ExperimentMetric— attaches a metric to an experiment with anexpected_direction; unique per (experiment, metric).MetricResultSnapshot— freezes computed results once an experiment completes.Experimentgainsexposure_event(default$flag_exposure) andcontrol_variant.API (gated on
EXPERIMENT_FLAG+ environment admin)…/environments/{key}/experiment-metrics/— metric library: list / create / retrieve / delete. (Notmetrics/— that path is taken by the usage-metrics viewset.) Deletion is blocked while attached to an active experiment.…/experiments/{id}/metrics/— attach / list / detach, with same-environment + unique-attach validation.…/experiments/{id}/results/— per-metric per-variantn/mean/variance, relative lift, confidence interval, and a per-metric verdict. Cached to a snapshot once completed.Results engine
query.pybuilds theassignment(argMinfirst-touch on$flag_exposure.value) +metricCTEs from a metric definition. Untrusted values are bound params;LEFT JOIN … coalesce(…,0)keeps assigned-but-inactive identities as real zeros.stats.pycompares variants with a Welch/z two-sample test (CI included; for a 0/1 conversion column this reduces to a two-proportion z-test).Scope notes (intentional cuts for v1)
query.py. A per-event id in the ingest stream is the clean long-term fix.Testing
experimentationsuite green (191 passed);mypyclean; migrations complete.🤖 Generated with Claude Code