-
Notifications
You must be signed in to change notification settings - Fork 528
feat(experimentation): environment-scoped metrics & experiment results #7674
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?
Changes from all commits
c961486
dd1f915
620f7b4
a48959c
cb1dbe8
b46b209
1fd7efb
de3e8b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,15 @@ | ||
| from rest_framework.routers import DefaultRouter | ||
| from rest_framework_nested import routers # type: ignore[import-untyped] | ||
|
|
||
| from experimentation.views import ExperimentViewSet | ||
| from experimentation.views import ExperimentMetricViewSet, ExperimentViewSet | ||
|
|
||
| app_name = "experiments" | ||
|
|
||
| router = DefaultRouter() | ||
| router = routers.DefaultRouter() | ||
| router.register(r"", ExperimentViewSet, basename="experiments") | ||
|
|
||
| urlpatterns = router.urls | ||
| experiments_router = routers.NestedSimpleRouter(router, r"", lookup="experiment") | ||
| experiments_router.register( | ||
| r"metrics", ExperimentMetricViewSet, basename="experiment-metrics" | ||
| ) | ||
|
|
||
| urlpatterns = router.urls + experiments_router.urls |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| """Versioned schemas for ``Metric.definition``. | ||
|
|
||
| ``definition`` is a schema-less JSON column whose shape is versioned so it can | ||
| evolve without breaking stored rows. Each supported version has a validator | ||
| here; the client sends the version it built the definition with. To introduce a | ||
| new shape, add an entry to ``METRIC_DEFINITION_VALIDATORS``. | ||
| """ | ||
|
|
||
| from collections.abc import Callable | ||
|
|
||
| DefinitionValidator = Callable[[dict[str, object]], "str | None"] | ||
|
|
||
|
|
||
| def _validate_v1(definition: dict[str, object]) -> str | None: | ||
| event = definition.get("event") | ||
| if not event or not isinstance(event, str): | ||
| return "Definition must specify a non-empty 'event'." | ||
| return None | ||
|
|
||
|
|
||
| METRIC_DEFINITION_VALIDATORS: dict[int, DefinitionValidator] = { | ||
| 1: _validate_v1, | ||
| } | ||
|
|
||
|
|
||
| def validate_metric_definition(definition: object) -> str | None: | ||
| """Return an error message if ``definition`` is invalid, else ``None``.""" | ||
| if not isinstance(definition, dict): | ||
| return "Definition must be an object." | ||
|
|
||
| version = definition.get("version") | ||
| validator = ( | ||
| METRIC_DEFINITION_VALIDATORS.get(version) if isinstance(version, int) else None | ||
| ) | ||
| if validator is None: | ||
| return "Definition must specify a supported 'version'." | ||
|
|
||
| return validator(definition) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from rest_framework.routers import DefaultRouter | ||
|
|
||
| from experimentation.views import MetricViewSet | ||
|
|
||
| app_name = "experiment_metrics" | ||
|
|
||
| router = DefaultRouter() | ||
| router.register(r"", MetricViewSet, basename="metrics") | ||
|
|
||
| urlpatterns = router.urls |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| # Generated by Django 5.2.14 on 2026-06-02 10:47 | ||
|
|
||
| import django.db.models.deletion | ||
| import uuid | ||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("environments", "0037_add_uuid_field"), | ||
| ("experimentation", "0004_experiment"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="Metric", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| models.AutoField( | ||
| auto_created=True, | ||
| primary_key=True, | ||
| serialize=False, | ||
| verbose_name="ID", | ||
| ), | ||
| ), | ||
| ( | ||
| "deleted_at", | ||
| models.DateTimeField( | ||
| blank=True, | ||
| db_index=True, | ||
| default=None, | ||
| editable=False, | ||
| null=True, | ||
| ), | ||
| ), | ||
| ( | ||
| "uuid", | ||
| models.UUIDField(default=uuid.uuid4, editable=False, unique=True), | ||
| ), | ||
| ("name", models.CharField(max_length=255)), | ||
| ("description", models.TextField(blank=True, default="")), | ||
| ( | ||
| "aggregation", | ||
| models.CharField( | ||
| choices=[ | ||
| ("count", "Count"), | ||
| ("sum", "Sum"), | ||
| ("mean", "Mean"), | ||
| ("occurrence", "Occurrence (event happened at least once)"), | ||
| ], | ||
| default="mean", | ||
| max_length=20, | ||
| ), | ||
| ), | ||
| ( | ||
| "direction", | ||
| models.CharField( | ||
| choices=[ | ||
| ("up", "Higher is better"), | ||
| ("down", "Lower is better"), | ||
| ("informational", "Informational only"), | ||
| ], | ||
| default="up", | ||
| max_length=20, | ||
| ), | ||
| ), | ||
| ("definition", models.JSONField()), | ||
| ("created_at", models.DateTimeField(auto_now_add=True)), | ||
| ("updated_at", models.DateTimeField(auto_now=True)), | ||
| ( | ||
| "environment", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="metrics", | ||
| to="environments.environment", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "abstract": False, | ||
| }, | ||
| ), | ||
| migrations.CreateModel( | ||
| name="ExperimentMetric", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| models.AutoField( | ||
| auto_created=True, | ||
| primary_key=True, | ||
| serialize=False, | ||
| verbose_name="ID", | ||
| ), | ||
| ), | ||
| ( | ||
| "expected_direction", | ||
| models.CharField( | ||
| choices=[ | ||
| ("increase", "Increase"), | ||
| ("decrease", "Decrease"), | ||
| ("not_increase", "Should not increase"), | ||
| ("not_decrease", "Should not decrease"), | ||
| ], | ||
| max_length=20, | ||
| ), | ||
| ), | ||
| ("created_at", models.DateTimeField(auto_now_add=True)), | ||
| ( | ||
| "experiment", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="experiment_metrics", | ||
| to="experimentation.experiment", | ||
| ), | ||
| ), | ||
| ( | ||
| "metric", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="experiment_metrics", | ||
| to="experimentation.metric", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "constraints": [ | ||
| models.UniqueConstraint( | ||
| fields=("experiment", "metric"), | ||
| name="metric_attached_once_per_experiment", | ||
| ) | ||
| ], | ||
| }, | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| add_environment_key_to_ingestion, | ||
| delete_environment_key_from_ingestion, | ||
| ) | ||
| from experimentation.types import MetricDefinition | ||
|
|
||
|
|
||
| class WarehouseType(models.TextChoices): | ||
|
|
@@ -117,3 +118,78 @@ class Meta: | |
| name="unique_active_experiment_per_feature_env", | ||
| ), | ||
| ] | ||
|
|
||
|
|
||
| class MetricAggregation(models.TextChoices): | ||
| COUNT = "count", "Count" | ||
| SUM = "sum", "Sum" | ||
| MEAN = "mean", "Mean" | ||
| OCCURRENCE = "occurrence", "Occurrence (event happened at least once)" | ||
|
|
||
|
|
||
| class MetricDirection(models.TextChoices): | ||
| """A metric's inherent polarity — which way is "better".""" | ||
|
|
||
| UP = "up", "Higher is better" | ||
| DOWN = "down", "Lower is better" | ||
| INFORMATIONAL = "informational", "Informational only" | ||
|
|
||
|
|
||
| class ExpectedDirection(models.TextChoices): | ||
| """The guardrail direction expected of a metric within an experiment.""" | ||
|
|
||
| INCREASE = "increase", "Increase" | ||
| DECREASE = "decrease", "Decrease" | ||
| NOT_INCREASE = "not_increase", "Should not increase" | ||
| NOT_DECREASE = "not_decrease", "Should not decrease" | ||
|
|
||
|
|
||
| class Metric(SoftDeleteExportableModel): | ||
| environment = models.ForeignKey( | ||
| Environment, | ||
| on_delete=models.CASCADE, | ||
| related_name="metrics", | ||
| ) | ||
| name = models.CharField(max_length=255) | ||
| description = models.TextField(blank=True, default="") | ||
| aggregation = models.CharField( | ||
| max_length=20, | ||
| choices=MetricAggregation.choices, | ||
| default=MetricAggregation.MEAN, | ||
| ) | ||
| direction = models.CharField( | ||
| max_length=20, | ||
| choices=MetricDirection.choices, | ||
| default=MetricDirection.UP, | ||
| ) | ||
| definition: models.JSONField[MetricDefinition, MetricDefinition] = ( | ||
| models.JSONField() | ||
| ) | ||
| created_at = models.DateTimeField(auto_now_add=True) | ||
| updated_at = models.DateTimeField(auto_now=True) | ||
|
|
||
|
|
||
| class ExperimentMetric(models.Model): | ||
| experiment = models.ForeignKey( | ||
| Experiment, | ||
| on_delete=models.CASCADE, | ||
| related_name="experiment_metrics", | ||
| ) | ||
| metric = models.ForeignKey( | ||
| Metric, | ||
| on_delete=models.CASCADE, | ||
| related_name="experiment_metrics", | ||
| ) | ||
| expected_direction = models.CharField( | ||
| max_length=20, | ||
| choices=ExpectedDirection.choices, | ||
| ) | ||
|
Comment on lines
+183
to
+186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I think i'm actually mixing 2 things:
|
||
| created_at = models.DateTimeField(auto_now_add=True) | ||
|
|
||
| class Meta: | ||
| constraints = [ | ||
| models.UniqueConstraint( | ||
| fields=["experiment", "metric"], | ||
| name="metric_attached_once_per_experiment", | ||
| ), | ||
| ] | ||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was the idea