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.

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: Added code for handling internal events #20995

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions posthog/tasks/test/__snapshots__/test_usage_report.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
count(distinct toDate(timestamp), event, cityHash64(distinct_id), cityHash64(uuid)) as count
FROM events
WHERE timestamp between '2022-01-10 00:00:00' AND '2022-01-10 23:59:59'
AND event != '$feature_flag_called'
AND event NOT IN ('survey sent',
AND event NOT IN ('$feature_flag_called',
'survey sent',
'survey shown',
'survey dismissed')
AND NOT startsWith(event, '$$')
GROUP BY team_id
'''
# ---
Expand Down Expand Up @@ -199,10 +200,11 @@
count(1) as count
FROM events
WHERE timestamp between '2022-01-01 00:00:00' AND '2022-01-10 23:59:59'
AND event != '$feature_flag_called'
AND event NOT IN ('survey sent',
AND event NOT IN ('$feature_flag_called',
'survey sent',
'survey shown',
'survey dismissed')
AND NOT startsWith(event, '$$')
GROUP BY team_id
'''
# ---
Expand Down
34 changes: 28 additions & 6 deletions posthog/tasks/test/test_usage_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,34 @@ def test_unlicensed_usage_report(self, mock_post: MagicMock, mock_client: MagicM
assert mock_posthog.capture.call_count == 2
mock_posthog.capture.assert_has_calls(calls, any_order=True)

@freeze_time("2022-01-10T00:01:00Z")
@patch("os.environ", {"DEPLOYMENT": "tests"})
@patch("posthog.tasks.usage_report.Client")
@patch("requests.post")
def test_does_not_count_internal_events(self, mock_post: MagicMock, mock_client: MagicMock) -> None:
distinct_id = str(uuid4())
_create_person(distinct_ids=[distinct_id], team=self.team)

_create_event(
distinct_id=distinct_id,
event="$pageview",
timestamp=now() - relativedelta(hours=12),
team=self.team,
)

_create_event(
distinct_id=distinct_id,
event="$$internal_event",
timestamp=now() - relativedelta(hours=12),
team=self.team,
)

period = get_previous_day()
period_start, period_end = period
all_reports = _get_all_org_reports(period_start, period_end)

assert all_reports[str(self.organization.id)].event_count_in_period == 1


class HogQLUsageReport(APIBaseTest, ClickhouseTestMixin, ClickhouseDestroyTablesMixin):
def test_usage_report_hogql_queries(self) -> None:
Expand Down Expand Up @@ -1140,12 +1168,6 @@ def setUp(self) -> None:
distinct_id=1,
timestamp="2021-10-09T13:01:01Z",
)
_create_event(
event="$$internal_metrics_shouldnt_be_billed",
team=self.team,
distinct_id=1,
timestamp="2021-10-09T13:01:01Z",
)
_create_event(
event="$pageview",
team=self.team2,
Expand Down
2 changes: 1 addition & 1 deletion posthog/tasks/usage_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def get_teams_with_billable_event_count_in_period(
f"""
SELECT team_id, count({distinct_expression}) as count
FROM events
WHERE timestamp between %(begin)s AND %(end)s AND event != '$feature_flag_called' AND event NOT IN ('survey sent', 'survey shown', 'survey dismissed')
WHERE timestamp between %(begin)s AND %(end)s AND event NOT IN ('$feature_flag_called', 'survey sent', 'survey shown', 'survey dismissed') AND NOT startsWith(event, '$$')
Copy link
Collaborator

Choose a reason for hiding this comment

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

flyby to make sure you test this in metabase for 1 day & 30 days of data, somewhat sure 30 days wouldn't work, but 1 day should be atmost 2x slower?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't tested it yet. Just started the PR but good call I'll have to see if it is at all reasonable

GROUP BY team_id
""",
{"begin": begin, "end": end},
Expand Down
Loading