From 711104254914e9b8ca13c3f0a6db3f19815ed13d Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Mon, 6 Jan 2025 05:10:39 -0800 Subject: [PATCH] feat(experiments): Store stats version on Experiment model (#27146) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- ee/clickhouse/views/experiments.py | 2 ++ .../views/test/test_clickhouse_experiments.py | 9 ++++-- frontend/src/queries/schema.json | 6 ---- frontend/src/queries/schema.ts | 2 -- .../experiments/ExperimentView/Info.tsx | 28 +++++++++++++++++-- .../scenes/experiments/experimentLogic.tsx | 7 +++++ frontend/src/types.ts | 3 ++ .../test/__snapshots__/test_feature_flag.ambr | 3 +- .../test_organization_feature_flag.ambr | 3 +- .../experiment_funnels_query_runner.py | 2 +- .../experiment_trends_query_runner.py | 2 +- .../test_experiment_funnels_query_runner.py | 3 +- .../test_experiment_trends_query_runner.py | 6 ++-- .../0538_experiment_stats_config.py | 17 +++++++++++ posthog/migrations/max_migration.txt | 2 +- posthog/models/experiment.py | 5 ++++ posthog/schema.py | 2 -- .../test_process_scheduled_changes.ambr | 6 ++-- 18 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 posthog/migrations/0538_experiment_stats_config.py diff --git a/ee/clickhouse/views/experiments.py b/ee/clickhouse/views/experiments.py index 9a21a668c273a..e3f45d028f84a 100644 --- a/ee/clickhouse/views/experiments.py +++ b/ee/clickhouse/views/experiments.py @@ -192,6 +192,7 @@ class Meta: "type", "metrics", "metrics_secondary", + "stats_config", ] read_only_fields = [ "id", @@ -376,6 +377,7 @@ def update(self, instance: Experiment, validated_data: dict, *args: Any, **kwarg "holdout", "metrics", "metrics_secondary", + "stats_config", } given_keys = set(validated_data.keys()) extra_keys = given_keys - expected_keys diff --git a/ee/clickhouse/views/test/test_clickhouse_experiments.py b/ee/clickhouse/views/test/test_clickhouse_experiments.py index 4501301e3befd..a183345d6f991 100644 --- a/ee/clickhouse/views/test/test_clickhouse_experiments.py +++ b/ee/clickhouse/views/test/test_clickhouse_experiments.py @@ -102,6 +102,11 @@ def test_creating_updating_basic_experiment(self): self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(response.json()["name"], "Test Experiment") self.assertEqual(response.json()["feature_flag_key"], ff_key) + self.assertEqual(response.json()["stats_config"], {}) + + id = response.json()["id"] + experiment = Experiment.objects.get(pk=id) + self.assertEqual(experiment.get_stats_config("version"), None) created_ff = FeatureFlag.objects.get(key=ff_key) @@ -110,13 +115,12 @@ def test_creating_updating_basic_experiment(self): self.assertEqual(created_ff.filters["multivariate"]["variants"][1]["key"], "test") self.assertEqual(created_ff.filters["groups"][0]["properties"], []) - id = response.json()["id"] end_date = "2021-12-10T00:00" # Now update response = self.client.patch( f"/api/projects/{self.team.id}/experiments/{id}", - {"description": "Bazinga", "end_date": end_date}, + {"description": "Bazinga", "end_date": end_date, "stats_config": {"version": 2}}, ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -124,6 +128,7 @@ def test_creating_updating_basic_experiment(self): experiment = Experiment.objects.get(pk=id) self.assertEqual(experiment.description, "Bazinga") self.assertEqual(experiment.end_date.strftime("%Y-%m-%dT%H:%M"), end_date) + self.assertEqual(experiment.get_stats_config("version"), 2) def test_creating_updating_web_experiment(self): ff_key = "a-b-tests" diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index adf0ef5f77413..dc8c5f7d58221 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -6017,9 +6017,6 @@ }, "response": { "$ref": "#/definitions/ExperimentFunnelsQueryResponse" - }, - "stats_version": { - "type": "integer" } }, "required": ["funnels_query", "kind"], @@ -6121,9 +6118,6 @@ }, "response": { "$ref": "#/definitions/ExperimentTrendsQueryResponse" - }, - "stats_version": { - "type": "integer" } }, "required": ["count_query", "kind"], diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 26b995a1c6d1d..788793f20755f 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -2052,7 +2052,6 @@ export interface ExperimentFunnelsQuery extends DataNode { @@ -2063,7 +2062,6 @@ export interface ExperimentTrendsQuery extends DataNode } + const currentStatsVersion = experiment.stats_config?.version || 1 + return (
@@ -72,6 +74,28 @@ export function Info(): JSX.Element {
)} + {featureFlags[FEATURE_FLAGS.EXPERIMENT_STATS_V2] && ( +
+
+ Stats Version +
+
+ {[1, 2].map((version) => ( + { + setExperimentStatsVersion(version) + }} + > + v{version} + + ))} +
+
+ )}
diff --git a/frontend/src/scenes/experiments/experimentLogic.tsx b/frontend/src/scenes/experiments/experimentLogic.tsx index ad419ae764e9c..a3c3414ee761c 100644 --- a/frontend/src/scenes/experiments/experimentLogic.tsx +++ b/frontend/src/scenes/experiments/experimentLogic.tsx @@ -191,6 +191,7 @@ export const experimentLogic = kea([ updateExperimentGoal: true, updateExperimentCollectionGoal: true, changeExperimentStartDate: (startDate: string) => ({ startDate }), + setExperimentStatsVersion: (version: number) => ({ version }), launchExperiment: true, endExperiment: true, addExperimentGroup: true, @@ -695,6 +696,12 @@ export const experimentLogic = kea([ actions.updateExperiment({ start_date: startDate }) values.experiment && eventUsageLogic.actions.reportExperimentStartDateChange(values.experiment, startDate) }, + setExperimentStatsVersion: async ({ version }, breakpoint) => { + actions.updateExperiment({ stats_config: { version } }) + await breakpoint(100) + actions.loadMetricResults(true) + actions.loadSecondaryMetricResults(true) + }, endExperiment: async () => { const endDate = dayjs() actions.updateExperiment({ end_date: endDate.toISOString() }) diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 91062d3976ab5..9f93e72422fd5 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -3332,6 +3332,9 @@ export interface Experiment { updated_at: string | null holdout_id?: number | null holdout?: Holdout + stats_config?: { + version?: number + } } export interface FunnelExperimentVariant { diff --git a/posthog/api/test/__snapshots__/test_feature_flag.ambr b/posthog/api/test/__snapshots__/test_feature_flag.ambr index 63dd2c1ae434a..00c8687baaf04 100644 --- a/posthog/api/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_feature_flag.ambr @@ -1572,7 +1572,8 @@ "posthog_experiment"."type", "posthog_experiment"."variants", "posthog_experiment"."metrics", - "posthog_experiment"."metrics_secondary" + "posthog_experiment"."metrics_secondary", + "posthog_experiment"."stats_config" FROM "posthog_experiment" WHERE "posthog_experiment"."exposure_cohort_id" = 99999 ''' diff --git a/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr b/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr index 23980835406eb..7cfe2c7cf7e7a 100644 --- a/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr @@ -1857,7 +1857,8 @@ "posthog_experiment"."type", "posthog_experiment"."variants", "posthog_experiment"."metrics", - "posthog_experiment"."metrics_secondary" + "posthog_experiment"."metrics_secondary", + "posthog_experiment"."stats_config" FROM "posthog_experiment" WHERE "posthog_experiment"."feature_flag_id" = 99999 ''' diff --git a/posthog/hogql_queries/experiments/experiment_funnels_query_runner.py b/posthog/hogql_queries/experiments/experiment_funnels_query_runner.py index 08c7e3dd91de5..cd8203b5dbbf9 100644 --- a/posthog/hogql_queries/experiments/experiment_funnels_query_runner.py +++ b/posthog/hogql_queries/experiments/experiment_funnels_query_runner.py @@ -51,7 +51,7 @@ def __init__(self, *args, **kwargs): if self.experiment.holdout: self.variants.append(f"holdout-{self.experiment.holdout.id}") - self.stats_version = self.query.stats_version or 1 + self.stats_version = self.experiment.get_stats_config("version") or 1 self.prepared_funnels_query = self._prepare_funnel_query() self.funnels_query_runner = FunnelsQueryRunner( diff --git a/posthog/hogql_queries/experiments/experiment_trends_query_runner.py b/posthog/hogql_queries/experiments/experiment_trends_query_runner.py index b3c72c788c951..a5c86f5fff9a8 100644 --- a/posthog/hogql_queries/experiments/experiment_trends_query_runner.py +++ b/posthog/hogql_queries/experiments/experiment_trends_query_runner.py @@ -66,7 +66,7 @@ def __init__(self, *args, **kwargs): self.variants.append(f"holdout-{self.experiment.holdout.id}") self.breakdown_key = f"$feature/{self.feature_flag.key}" - self.stats_version = self.query.stats_version or 1 + self.stats_version = self.experiment.get_stats_config("version") or 1 self.prepared_count_query = self._prepare_count_query() self.prepared_exposure_query = self._prepare_exposure_query() diff --git a/posthog/hogql_queries/experiments/test/test_experiment_funnels_query_runner.py b/posthog/hogql_queries/experiments/test/test_experiment_funnels_query_runner.py index 3dc11499b1393..1da3e194bb838 100644 --- a/posthog/hogql_queries/experiments/test/test_experiment_funnels_query_runner.py +++ b/posthog/hogql_queries/experiments/test/test_experiment_funnels_query_runner.py @@ -141,6 +141,8 @@ def test_query_runner(self): def test_query_runner_v2(self): feature_flag = self.create_feature_flag() experiment = self.create_experiment(feature_flag=feature_flag) + experiment.stats_config = {"version": 2} + experiment.save() feature_flag_property = f"$feature/{feature_flag.key}" @@ -152,7 +154,6 @@ def test_query_runner_v2(self): experiment_id=experiment.id, kind="ExperimentFunnelsQuery", funnels_query=funnels_query, - stats_version=2, ) experiment.metrics = [{"type": "primary", "query": experiment_query.model_dump()}] diff --git a/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py b/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py index 0d3b37eeedc77..cca70f6a26847 100644 --- a/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py +++ b/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py @@ -1287,6 +1287,8 @@ def test_query_runner_with_avg_math(self): def test_query_runner_with_avg_math_v2_stats(self): feature_flag = self.create_feature_flag() experiment = self.create_experiment(feature_flag=feature_flag) + experiment.stats_config = {"version": 2} + experiment.save() feature_flag_property = f"$feature/{feature_flag.key}" @@ -1302,7 +1304,6 @@ def test_query_runner_with_avg_math_v2_stats(self): kind="ExperimentTrendsQuery", count_query=count_query, exposure_query=exposure_query, - stats_version=2, ) experiment.metrics = [{"type": "primary", "query": experiment_query.model_dump()}] @@ -1522,6 +1523,8 @@ def test_query_runner_standard_flow(self): def test_query_runner_standard_flow_v2_stats(self): feature_flag = self.create_feature_flag() experiment = self.create_experiment(feature_flag=feature_flag) + experiment.stats_config = {"version": 2} + experiment.save() ff_property = f"$feature/{feature_flag.key}" count_query = TrendsQuery(series=[EventsNode(event="$pageview")]) @@ -1532,7 +1535,6 @@ def test_query_runner_standard_flow_v2_stats(self): kind="ExperimentTrendsQuery", count_query=count_query, exposure_query=exposure_query, - stats_version=2, ) experiment.metrics = [{"type": "primary", "query": experiment_query.model_dump()}] diff --git a/posthog/migrations/0538_experiment_stats_config.py b/posthog/migrations/0538_experiment_stats_config.py new file mode 100644 index 0000000000000..f6b0aa39471d3 --- /dev/null +++ b/posthog/migrations/0538_experiment_stats_config.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.15 on 2025-01-06 12:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("posthog", "0537_data_color_themes"), + ] + + operations = [ + migrations.AddField( + model_name="experiment", + name="stats_config", + field=models.JSONField(blank=True, default=dict, null=True), + ), + ] diff --git a/posthog/migrations/max_migration.txt b/posthog/migrations/max_migration.txt index 5224bd35001ac..d68ffae602bc4 100644 --- a/posthog/migrations/max_migration.txt +++ b/posthog/migrations/max_migration.txt @@ -1 +1 @@ -0537_data_color_themes +0538_experiment_stats_config diff --git a/posthog/models/experiment.py b/posthog/models/experiment.py index 87119d292b689..eeb59ed9eae73 100644 --- a/posthog/models/experiment.py +++ b/posthog/models/experiment.py @@ -46,9 +46,14 @@ class ExperimentType(models.TextChoices): "ExperimentSavedMetric", blank=True, related_name="experiments", through="ExperimentToSavedMetric" ) + stats_config = models.JSONField(default=dict, null=True, blank=True) + def get_feature_flag_key(self): return self.feature_flag.key + def get_stats_config(self, key: str): + return self.stats_config.get(key) if self.stats_config else None + @property def is_draft(self): return not self.start_date diff --git a/posthog/schema.py b/posthog/schema.py index 77561f637c7b9..1dddae4e22282 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -6606,7 +6606,6 @@ class ExperimentTrendsQuery(BaseModel): ) name: Optional[str] = None response: Optional[ExperimentTrendsQueryResponse] = None - stats_version: Optional[int] = None class FunnelPathsFilter(BaseModel): @@ -6723,7 +6722,6 @@ class ExperimentFunnelsQuery(BaseModel): ) name: Optional[str] = None response: Optional[ExperimentFunnelsQueryResponse] = None - stats_version: Optional[int] = None class FunnelCorrelationQuery(BaseModel): diff --git a/posthog/tasks/test/__snapshots__/test_process_scheduled_changes.ambr b/posthog/tasks/test/__snapshots__/test_process_scheduled_changes.ambr index a16e3b07ba1ab..b0d029a734b8a 100644 --- a/posthog/tasks/test/__snapshots__/test_process_scheduled_changes.ambr +++ b/posthog/tasks/test/__snapshots__/test_process_scheduled_changes.ambr @@ -557,7 +557,8 @@ "posthog_experiment"."type", "posthog_experiment"."variants", "posthog_experiment"."metrics", - "posthog_experiment"."metrics_secondary" + "posthog_experiment"."metrics_secondary", + "posthog_experiment"."stats_config" FROM "posthog_experiment" WHERE "posthog_experiment"."feature_flag_id" = 99999 ''' @@ -1186,7 +1187,8 @@ "posthog_experiment"."type", "posthog_experiment"."variants", "posthog_experiment"."metrics", - "posthog_experiment"."metrics_secondary" + "posthog_experiment"."metrics_secondary", + "posthog_experiment"."stats_config" FROM "posthog_experiment" WHERE "posthog_experiment"."feature_flag_id" = 99999 '''