Skip to content

Commit

Permalink
feat(experiments): implement reusable metrics (#27154)
Browse files Browse the repository at this point in the history
  • Loading branch information
jurajmajerik authored Dec 27, 2024
1 parent 4797290 commit 6c58e55
Show file tree
Hide file tree
Showing 37 changed files with 1,624 additions and 184 deletions.
17 changes: 11 additions & 6 deletions ee/clickhouse/views/experiment_saved_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
from posthog.api.routing import TeamAndOrgViewSetMixin
from posthog.api.shared import UserBasicSerializer
from posthog.models.experiment import ExperimentSavedMetric, ExperimentToSavedMetric
from posthog.schema import FunnelsQuery, TrendsQuery
from posthog.schema import ExperimentFunnelsQuery, ExperimentTrendsQuery


class ExperimentToSavedMetricSerializer(serializers.ModelSerializer):
query = serializers.JSONField(source="saved_metric.query", read_only=True)
name = serializers.CharField(source="saved_metric.name", read_only=True)

class Meta:
model = ExperimentToSavedMetric
fields = [
Expand All @@ -18,6 +21,8 @@ class Meta:
"saved_metric",
"metadata",
"created_at",
"query",
"name",
]
read_only_fields = [
"id",
Expand Down Expand Up @@ -52,15 +57,15 @@ def validate_query(self, value):

metric_query = value

if metric_query.get("kind") not in ["TrendsQuery", "FunnelsQuery"]:
raise ValidationError("Metric query kind must be 'TrendsQuery' or 'FunnelsQuery'")
if metric_query.get("kind") not in ["ExperimentTrendsQuery", "ExperimentFunnelsQuery"]:
raise ValidationError("Metric query kind must be 'ExperimentTrendsQuery' or 'ExperimentFunnelsQuery'")

# pydantic models are used to validate the query
try:
if metric_query["kind"] == "TrendsQuery":
TrendsQuery(**metric_query)
if metric_query["kind"] == "ExperimentTrendsQuery":
ExperimentTrendsQuery(**metric_query)
else:
FunnelsQuery(**metric_query)
ExperimentFunnelsQuery(**metric_query)
except pydantic.ValidationError as e:
raise ValidationError(str(e.errors())) from e

Expand Down
4 changes: 1 addition & 3 deletions ee/clickhouse/views/experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ class ExperimentSerializer(serializers.ModelSerializer):
queryset=ExperimentHoldout.objects.all(), source="holdout", required=False, allow_null=True
)
saved_metrics = ExperimentToSavedMetricSerializer(many=True, source="experimenttosavedmetric_set", read_only=True)
saved_metrics_ids = serializers.ListField(
child=serializers.JSONField(), write_only=True, required=False, allow_null=True
)
saved_metrics_ids = serializers.ListField(child=serializers.JSONField(), required=False, allow_null=True)

class Meta:
model = Experiment
Expand Down
29 changes: 24 additions & 5 deletions ee/clickhouse/views/test/test_clickhouse_experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,13 @@ def test_saved_metrics(self):
{
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {
"kind": "TrendsQuery",
"series": [{"kind": "EventsNode", "event": "$pageview"}],
},
},
},
)

Expand All @@ -380,7 +386,10 @@ def test_saved_metrics(self):
self.assertEqual(response.json()["description"], "Test description")
self.assertEqual(
response.json()["query"],
{"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
)
self.assertEqual(response.json()["created_by"]["id"], self.user.pk)

Expand Down Expand Up @@ -418,7 +427,11 @@ def test_saved_metrics(self):
saved_metric = Experiment.objects.get(pk=exp_id).saved_metrics.first()
self.assertEqual(saved_metric.id, saved_metric_id)
self.assertEqual(
saved_metric.query, {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]}
saved_metric.query,
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
)

# Now try updating experiment with new saved metric
Expand All @@ -427,7 +440,10 @@ def test_saved_metrics(self):
{
"name": "Test Experiment saved metric 2",
"description": "Test description 2",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
},
},
)

Expand Down Expand Up @@ -513,7 +529,10 @@ def test_validate_saved_metrics_payload(self):
{
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
},
)

Expand Down
63 changes: 48 additions & 15 deletions ee/clickhouse/views/test/test_experiment_saved_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ def test_validation_of_query_metric(self):
)

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json()["detail"], "Metric query kind must be 'TrendsQuery' or 'FunnelsQuery'")
self.assertEqual(
response.json()["detail"], "Metric query kind must be 'ExperimentTrendsQuery' or 'ExperimentFunnelsQuery'"
)

response = self.client.post(
f"/api/projects/{self.team.id}/experiment_saved_metrics/",
Expand All @@ -47,40 +49,46 @@ def test_validation_of_query_metric(self):
)

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json()["detail"], "Metric query kind must be 'TrendsQuery' or 'FunnelsQuery'")
self.assertEqual(
response.json()["detail"], "Metric query kind must be 'ExperimentTrendsQuery' or 'ExperimentFunnelsQuery'"
)

response = self.client.post(
f"/api/projects/{self.team.id}/experiment_saved_metrics/",
data={
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "ExperimentTrendsQuery"},
"query": {"kind": "TrendsQuery"},
},
format="json",
)

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json()["detail"], "Metric query kind must be 'TrendsQuery' or 'FunnelsQuery'")
self.assertEqual(
response.json()["detail"], "Metric query kind must be 'ExperimentTrendsQuery' or 'ExperimentFunnelsQuery'"
)

response = self.client.post(
f"/api/projects/{self.team.id}/experiment_saved_metrics/",
data={
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "TrendsQuery"},
"query": {"kind": "ExperimentTrendsQuery"},
},
format="json",
)

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertTrue("'loc': ('series',), 'msg': 'Field required'" in response.json()["detail"])
self.assertTrue("'loc': ('count_query',), 'msg': 'Field required'" in response.json()["detail"])

response = self.client.post(
f"/api/projects/{self.team.id}/experiment_saved_metrics/",
data={
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
},
format="json",
)
Expand All @@ -93,7 +101,13 @@ def test_create_update_experiment_saved_metrics(self) -> None:
data={
"name": "Test Experiment saved metric",
"description": "Test description",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {
"kind": "TrendsQuery",
"series": [{"kind": "EventsNode", "event": "$pageview"}],
},
},
},
format="json",
)
Expand All @@ -104,7 +118,10 @@ def test_create_update_experiment_saved_metrics(self) -> None:
self.assertEqual(response.json()["description"], "Test description")
self.assertEqual(
response.json()["query"],
{"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
)
self.assertEqual(response.json()["created_by"]["id"], self.user.pk)

Expand Down Expand Up @@ -142,7 +159,11 @@ def test_create_update_experiment_saved_metrics(self) -> None:
saved_metric = Experiment.objects.get(pk=exp_id).saved_metrics.first()
self.assertEqual(saved_metric.id, saved_metric_id)
self.assertEqual(
saved_metric.query, {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]}
saved_metric.query,
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
)

# Now try updating saved metric
Expand All @@ -151,15 +172,21 @@ def test_create_update_experiment_saved_metrics(self) -> None:
{
"name": "Test Experiment saved metric 2",
"description": "Test description 2",
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
},
},
)

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["name"], "Test Experiment saved metric 2")
self.assertEqual(
response.json()["query"],
{"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
},
)

# make sure experiment in question was updated as well
Expand All @@ -168,7 +195,10 @@ def test_create_update_experiment_saved_metrics(self) -> None:
self.assertEqual(saved_metric.id, saved_metric_id)
self.assertEqual(
saved_metric.query,
{"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
{
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageleave"}]},
},
)
self.assertEqual(saved_metric.name, "Test Experiment saved metric 2")
self.assertEqual(saved_metric.description, "Test description 2")
Expand All @@ -186,7 +216,10 @@ def test_invalid_create(self):
f"/api/projects/{self.team.id}/experiment_saved_metrics/",
data={
"name": None, # invalid
"query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
"query": {
"kind": "ExperimentTrendsQuery",
"count_query": {"kind": "TrendsQuery", "series": [{"kind": "EventsNode", "event": "$pageview"}]},
},
},
format="json",
)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions frontend/src/scenes/appScenes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export const appScenes: Record<Scene, () => any> = {
[Scene.Group]: () => import('./groups/Group'),
[Scene.Action]: () => import('./actions/Action'),
[Scene.Experiments]: () => import('./experiments/Experiments'),
[Scene.ExperimentsSavedMetrics]: () => import('./experiments/SavedMetrics/SavedMetrics'),
[Scene.ExperimentsSavedMetric]: () => import('./experiments/SavedMetrics/SavedMetric'),
[Scene.Experiment]: () => import('./experiments/Experiment'),
[Scene.FeatureFlags]: () => import('./feature-flags/FeatureFlags'),
[Scene.FeatureManagement]: () => import('./feature-flags/FeatureManagement'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { BaseMathType, ChartDisplayType, InsightType, PropertyFilterType, Proper
import { experimentLogic } from '../experimentLogic'

export function CumulativeExposuresChart(): JSX.Element {
const { experiment, metricResults, getMetricType } = useValues(experimentLogic)
const { experiment, metricResults, _getMetricType } = useValues(experimentLogic)

const metricIdx = 0
const metricType = getMetricType(metricIdx)
const metricType = _getMetricType(experiment.metrics[metricIdx])
const result = metricResults?.[metricIdx]
const variants = experiment.parameters?.feature_flag_variants?.map((variant) => variant.key) || []
if (experiment.holdout) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ export function DataCollection(): JSX.Element {
const {
experimentId,
experiment,
getMetricType,
_getMetricType,
funnelResultsPersonsTotal,
actualRunningTime,
minimumDetectableEffect,
} = useValues(experimentLogic)

const { openExperimentCollectionGoalModal } = useActions(experimentLogic)

const metricType = getMetricType(0)
const metricType = _getMetricType(experiment.metrics[0])

const recommendedRunningTime = experiment?.parameters?.recommended_running_time || 1
const recommendedSampleSize = experiment?.parameters?.recommended_sample_size || 100
Expand Down Expand Up @@ -80,7 +80,7 @@ export function DataCollection(): JSX.Element {
<LemonProgress
className="w-full border"
bgColor="var(--bg-table)"
size="large"
size="medium"
percent={experimentProgressPercent}
/>
{metricType === InsightType.TRENDS && (
Expand Down Expand Up @@ -172,7 +172,8 @@ export function DataCollection(): JSX.Element {
export function DataCollectionGoalModal({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element {
const {
isExperimentCollectionGoalModalOpen,
getMetricType,
experiment,
_getMetricType,
trendMetricInsightLoading,
funnelMetricInsightLoading,
} = useValues(experimentLogic({ experimentId }))
Expand All @@ -181,7 +182,9 @@ export function DataCollectionGoalModal({ experimentId }: { experimentId: Experi
)

const isInsightLoading =
getMetricType(0) === InsightType.TRENDS ? trendMetricInsightLoading : funnelMetricInsightLoading
_getMetricType(experiment.metrics[0]) === InsightType.TRENDS
? trendMetricInsightLoading
: funnelMetricInsightLoading

return (
<LemonModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ function TrendCalculation({ experimentId }: ExperimentCalculatorProps): JSX.Elem
}

export function DataCollectionCalculator({ experimentId }: ExperimentCalculatorProps): JSX.Element {
const { getMetricType, minimumDetectableEffect, experiment, conversionMetrics } = useValues(
const { _getMetricType, minimumDetectableEffect, experiment, conversionMetrics } = useValues(
experimentLogic({ experimentId })
)
const { setExperiment } = useActions(experimentLogic({ experimentId }))

const metricType = getMetricType(0)
const metricType = _getMetricType(experiment.metrics[0])

// :KLUDGE: need these to mount the Query component to load the insight */
const insightLogicInstance = insightLogic({
Expand Down Expand Up @@ -217,7 +217,7 @@ export function DataCollectionCalculator({ experimentId }: ExperimentCalculatorP
The calculations are based on the events received in the last 14 days. This event count may
differ from what was considered in earlier estimates.
</LemonBanner>
{getMetricType(0) === InsightType.TRENDS ? (
{_getMetricType(experiment.metrics[0]) === InsightType.TRENDS ? (
<TrendCalculation experimentId={experimentId} />
) : (
<FunnelCalculation experimentId={experimentId} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { WebExperimentImplementationDetails } from 'scenes/experiments/WebExperi
import { ExperimentImplementationDetails } from '../ExperimentImplementationDetails'
import { experimentLogic } from '../experimentLogic'
import { MetricModal } from '../Metrics/MetricModal'
import { MetricSourceModal } from '../Metrics/MetricSourceModal'
import { SavedMetricModal } from '../Metrics/SavedMetricModal'
import { MetricsView } from '../MetricsView/MetricsView'
import {
ExperimentLoadingAnimation,
Expand Down Expand Up @@ -141,9 +143,15 @@ export function ExperimentView(): JSX.Element {
/>
</>
)}
<MetricSourceModal experimentId={experimentId} isSecondary={true} />
<MetricSourceModal experimentId={experimentId} isSecondary={false} />

<MetricModal experimentId={experimentId} isSecondary={true} />
<MetricModal experimentId={experimentId} isSecondary={false} />

<SavedMetricModal experimentId={experimentId} isSecondary={true} />
<SavedMetricModal experimentId={experimentId} isSecondary={false} />

<DistributionModal experimentId={experimentId} />
<ReleaseConditionsModal experimentId={experimentId} />
</>
Expand Down
Loading

0 comments on commit 6c58e55

Please sign in to comment.