Skip to content
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.

Sign up for GitHub

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

feat(experiments): implement reusable metrics #27154

Merged
merged 20 commits into from
Dec 27, 2024
Merged

feat(experiments): implement reusable metrics #27154

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
82a239c
init
jurajmajerik Dec 25, 2024
9fc4b53
merge
jurajmajerik Dec 25, 2024
c553266
Merge branch 'master' of https://github.com/PostHog/posthog into expe…
jurajmajerik Dec 25, 2024
a9706de
wip
jurajmajerik Dec 25, 2024
a15cd8d
wip
jurajmajerik Dec 26, 2024
c5a6549
wip
jurajmajerik Dec 26, 2024
bde7d45
wip
jurajmajerik Dec 26, 2024
e2bea11
update tests
jurajmajerik Dec 26, 2024
9ae7ca2
Update UI snapshots for `webkit` (2)
github-actions[bot] Dec 26, 2024
8c205d7
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 26, 2024
cd880cb
Update UI snapshots for `webkit` (2)
github-actions[bot] Dec 26, 2024
962e385
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 26, 2024
b5c0841
update tests
jurajmajerik Dec 26, 2024
2507e21
Merge branch 'experiments-saved-metrics' of https://github.com/PostHo…
jurajmajerik Dec 26, 2024
db323bd
wip
jurajmajerik Dec 26, 2024
bec4d0a
wip
jurajmajerik Dec 26, 2024
2c70f8b
Merge branch 'master' of https://github.com/PostHog/posthog into expe…
jurajmajerik Dec 26, 2024
d2835df
wip
jurajmajerik Dec 26, 2024
ab54a57
wip
jurajmajerik Dec 27, 2024
435c64d
wip
jurajmajerik Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions ee/clickhouse/views/experiment_saved_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
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):
Expand Down Expand Up @@ -52,15 +52,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
29 changes: 24 additions & 5 deletions ee/clickhouse/views/test/test_clickhouse_experiments.py
View file
Open in desktop
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
View file
Open in desktop
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
Binary file modified frontend/__snapshots__/scenes-app-insights--trends-line--light--webkit.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-insights--trends-line--light.png
View file
Open in desktop
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
View file
Open in desktop
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
17 changes: 15 additions & 2 deletions frontend/src/scenes/experiments/Experiments.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ExperimentsHog } from 'lib/components/hedgehogs'
import { MemberSelect } from 'lib/components/MemberSelect'
import { PageHeader } from 'lib/components/PageHeader'
import { ProductIntroduction } from 'lib/components/ProductIntroduction/ProductIntroduction'
import { FEATURE_FLAGS } from 'lib/constants'
import { dayjs } from 'lib/dayjs'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { More } from 'lib/lemon-ui/LemonButton/More'
Expand All @@ -23,15 +24,24 @@ import { Experiment, ExperimentsTabs, ProductKey, ProgressStatus } from '~/types
import { experimentsLogic, getExperimentStatus } from './experimentsLogic'
import { StatusTag } from './ExperimentView/components'
import { Holdouts } from './Holdouts'
import { SavedMetrics } from './SavedMetrics/SavedMetrics'

export const scene: SceneExport = {
component: Experiments,
logic: experimentsLogic,
}

export function Experiments(): JSX.Element {
const { filteredExperiments, experimentsLoading, tab, searchTerm, shouldShowEmptyState, searchStatus, userFilter } =
useValues(experimentsLogic)
const {
filteredExperiments,
experimentsLoading,
tab,
searchTerm,
shouldShowEmptyState,
searchStatus,
userFilter,
featureFlags,
} = useValues(experimentsLogic)
const { setExperimentsTab, deleteExperiment, archiveExperiment, setSearchStatus, setSearchTerm, setUserFilter } =
useActions(experimentsLogic)

Expand Down Expand Up @@ -211,11 +221,14 @@ export function Experiments(): JSX.Element {
{ key: ExperimentsTabs.Yours, label: 'Your experiments' },
{ key: ExperimentsTabs.Archived, label: 'Archived experiments' },
{ key: ExperimentsTabs.Holdouts, label: 'Holdout groups' },
{ key: ExperimentsTabs.SavedMetrics, label: 'Saved metrics' },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the tab show up without the flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will do!

]}
/>

{tab === ExperimentsTabs.Holdouts ? (
<Holdouts />
) : tab === ExperimentsTabs.SavedMetrics && featureFlags[FEATURE_FLAGS.EXPERIMENTS_MULTIPLE_METRICS] ? (
<SavedMetrics />
) : (
<>
{tab === ExperimentsTabs.Archived ? (
Expand Down
Loading
Loading