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

fix(experiments): Standardize trends cont. exposure query logic #27647

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+284 −108
Open

fix(experiments): Standardize trends cont. exposure query logic #27647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
036b792
Use standard exposure query logic for trends continuous
danielbachhuber Jan 17, 2025
dffa8fc
Pass `exposure_query=None` to better match prod behavior
danielbachhuber Jan 17, 2025
f463016
Don't pass an exposure query unless it's a custom query
danielbachhuber Jan 17, 2025
da2195e
Follow up from f4630160d2feba434ad4f772c4234dee96d95be4
danielbachhuber Jan 17, 2025
6561d8c
Merge branch 'master' into experiments/fix-trends-continuous-exposure
danielbachhuber Jan 17, 2025
eef1253
Update values for 036b792184279a78f2fdac14c069098989c0f7e8
danielbachhuber Jan 17, 2025
a6938b4
Ensure `filterTestAccounts` is applied to trends exposure queries
danielbachhuber Jan 17, 2025
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
64 changes: 7 additions & 57 deletions posthog/hogql_queries/experiments/experiment_trends_query_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,69 +169,18 @@ def _prepare_count_query(self) -> TrendsQuery:

def _prepare_exposure_query(self) -> TrendsQuery:
"""
This method prepares the exposure query for the experiment analysis.

Exposure is the count of users who have seen the experiment. This is necessary to calculate the statistical
significance of the experiment.

There are 3 possible cases for the exposure query:
1. If math aggregation is used, we construct an implicit exposure query
2. Otherwise, if an exposure query is provided, we use it as is, adapting it to the experiment's duration and breakdown
3. Otherwise, we construct a default exposure query (the count of $feature_flag_called events)
There are 2 possible cases for the exposure query:
1. Otherwise, if an exposure query is provided, we use it as is, adapting it to the experiment's duration and breakdown
2. Otherwise, we construct a default exposure query (the count of $feature_flag_called events)
"""

# 1. If math aggregation is used, we construct an implicit exposure query: unique users for the count event
uses_math_aggregation = self._uses_math_aggregation_by_user_or_property_value(self.query.count_query)
prepared_count_query = TrendsQuery(**self.query.count_query.model_dump())

if uses_math_aggregation:
prepared_exposure_query = prepared_count_query
prepared_exposure_query.dateRange = self._get_date_range()
prepared_exposure_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE)

# For a data warehouse query, we can use the unique users for the series
if self._is_data_warehouse_query(prepared_exposure_query):
prepared_exposure_query.breakdownFilter = self._get_data_warehouse_breakdown_filter()
prepared_exposure_query.series[0].math = BaseMathType.DAU
prepared_exposure_query.series[0].math_property = None
prepared_exposure_query.series[0].math_property_type = None
prepared_exposure_query.properties = [
DataWarehousePropertyFilter(
key="events.event",
value="$feature_flag_called",
operator=PropertyOperator.EXACT,
type="data_warehouse",
),
DataWarehousePropertyFilter(
key=f"events.properties.{self.breakdown_key}",
value=self.variants,
operator=PropertyOperator.EXACT,
type="data_warehouse",
),
]
else:
count_event = self.query.count_query.series[0]
if hasattr(count_event, "event"):
prepared_exposure_query.breakdownFilter = self._get_event_breakdown_filter()
prepared_exposure_query.series = [
EventsNode(
event=count_event.event,
math=BaseMathType.DAU,
)
]
prepared_exposure_query.properties = [
EventPropertyFilter(
key=self.breakdown_key,
value=self.variants,
operator=PropertyOperator.EXACT,
type="event",
)
]
else:
raise ValueError("Expected first series item to have an 'event' attribute")

# 2. Otherwise, if an exposure query is provided, we use it as is, adapting the date range and breakdown
elif self.query.exposure_query and not self._is_data_warehouse_query(prepared_count_query):
# 1. If an exposure query is provided, we use it as is, adapting it to the experiment's duration and breakdown
if self.query.exposure_query and not self._is_data_warehouse_query(prepared_count_query):
prepared_exposure_query = TrendsQuery(**self.query.exposure_query.model_dump())
prepared_exposure_query.dateRange = self._get_date_range()
prepared_exposure_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE)
Expand All @@ -244,7 +193,7 @@ def _prepare_exposure_query(self) -> TrendsQuery:
type="event",
)
]
# 3. Otherwise, we construct a default exposure query: unique users for the $feature_flag_called event
# 2. Otherwise, we construct a default exposure query: unique users for the $feature_flag_called event
else:
prepared_exposure_query = TrendsQuery(
dateRange=self._get_date_range(),
Expand Down Expand Up @@ -273,6 +222,7 @@ def _prepare_exposure_query(self) -> TrendsQuery:
type="event",
),
],
filterTestAccounts=self.query.count_query.filterTestAccounts,
)

return prepared_exposure_query
Expand Down
Loading
Loading