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: allow alternative recorder script names #25961

Merged
merged 15 commits into from
Nov 28, 2024
Merged

feat: allow alternative recorder script names #25961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
933bb5d
feat: allow alternative recorder script names
pauldambra Nov 2, 2024
673b4f8
parameterize tests
pauldambra Nov 2, 2024
58118ed
Merge branch 'master' into feat/allow-alternative-recorder-script-names
pauldambra Nov 2, 2024
6c153c8
Merge branch 'master' into feat/allow-alternative-recorder-script-names
pauldambra Nov 5, 2024
6128cd9
Merge branch 'master' into feat/allow-alternative-recorder-script-names
pauldambra Nov 28, 2024
9d026fe
Update query snapshots
github-actions[bot] Nov 28, 2024
343d93a
Update UI snapshots for `chromium` (1)
github-actions[bot] Nov 28, 2024
10e572c
Update UI snapshots for `chromium` (1)
github-actions[bot] Nov 28, 2024
162829b
fix
pauldambra Nov 28, 2024
532c770
Remove secondaryMetricsLogic.ts from PR
pauldambra Nov 28, 2024
2543060
fix
pauldambra Nov 28, 2024
d4d2823
fix wildcard check
pauldambra Nov 28, 2024
60d9c0a
Fix
pauldambra Nov 28, 2024
8551cc0
Update query snapshots
github-actions[bot] Nov 28, 2024
92e1d79
Merge branch 'master' into feat/allow-alternative-recorder-script-names
pauldambra Nov 28, 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
11 changes: 11 additions & 0 deletions posthog/api/decide.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,16 @@ def _session_recording_config_response(request: HttpRequest, team: Team, token:
else:
linked_flag = linked_flag_key

rrweb_script_config = None

if (settings.SESSION_REPLAY_RRWEB_SCRIPT is not None) and (
"*" in settings.SESSION_REPLAY_RRWEB_SCRIPT_ALLOWED_TEAMS
or str(team.id) in settings.SESSION_REPLAY_RRWEB_SCRIPT_ALLOWED_TEAMS
):
rrweb_script_config = {
"script": settings.SESSION_REPLAY_RRWEB_SCRIPT,
}
Comment on lines +368 to +370
Copy link
Member Author

Choose a reason for hiding this comment

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

slightly over careful in returing a structure rather than a value in case this changes over time e.g. loading from a different host


session_recording_config_response = {
"endpoint": "/s/",
"consoleLogRecordingEnabled": capture_console_logs,
Expand All @@ -370,6 +380,7 @@ def _session_recording_config_response(request: HttpRequest, team: Team, token:
"urlTriggers": team.session_recording_url_trigger_config,
"urlBlocklist": team.session_recording_url_blocklist_config,
"eventTriggers": team.session_recording_event_trigger_config,
"scriptConfig": rrweb_script_config,
}

if isinstance(team.session_replay_config, dict):
Expand Down
146 changes: 121 additions & 25 deletions posthog/api/test/test_decide.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import random
import time
from typing import Optional
from unittest.mock import patch
from unittest.mock import patch, Mock

import pytest
from django.conf import settings
Expand All @@ -13,6 +13,7 @@
from django.test import TestCase, TransactionTestCase
from django.test.client import Client
from freezegun import freeze_time
from parameterized import parameterized
from rest_framework import status
from rest_framework.test import APIClient

Expand Down Expand Up @@ -52,19 +53,23 @@
)


def make_session_recording_config(overrides: dict | None = None) -> dict:
def make_session_recording_decide_response(overrides: Optional[dict] = None) -> dict:
if overrides is None:
overrides = {}

return {
"endpoint": "/s/",
"recorderVersion": "v2",
"consoleLogRecordingEnabled": True,
"sampleRate": None,
"linkedFlag": None,
"minimumDurationMilliseconds": None,
"networkPayloadCapture": None,
"urlTriggers": [],
"urlBlocklist": [],
"scriptConfig": None,
"sampleRate": None,
"eventTriggers": [],
**(overrides or {}),
**overrides,
}


Expand Down Expand Up @@ -182,7 +187,7 @@ def test_user_session_recording_opt_in(self, *args):
self._update_team({"session_recording_opt_in": True})

response = self._post_decide().json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()
self.assertEqual(response["supportedCompression"], ["gzip", "gzip-js"])

def test_user_console_log_opt_in(self, *args):
Expand All @@ -193,7 +198,7 @@ def test_user_console_log_opt_in(self, *args):
self._update_team({"session_recording_opt_in": True, "capture_console_log_opt_in": True})

response = self._post_decide().json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

def test_user_performance_opt_in(self, *args):
# :TRICKY: Test for regression around caching
Expand Down Expand Up @@ -319,7 +324,7 @@ def test_session_recording_url_trigger_patterns(self, *args):
)

response = self._post_decide(origin="capacitor://localhost:8000/home").json()
assert response["sessionRecording"] == make_session_recording_config(
assert response["sessionRecording"] == make_session_recording_decide_response(
{
"urlTriggers": [{"url": "/replay-examples/", "matching": "regex"}],
}
Expand All @@ -334,7 +339,7 @@ def test_session_recording_url_blocklist_patterns(self, *args):
)

response = self._post_decide(origin="capacitor://localhost:8000/home").json()
assert response["sessionRecording"] == make_session_recording_config(
assert response["sessionRecording"] == make_session_recording_decide_response(
{
"urlBlocklist": [{"url": "/replay-examples/iframe", "matching": "regex"}],
}
Expand All @@ -349,7 +354,7 @@ def test_session_recording_event_triggers(self, *args):
)

response = self._post_decide(origin="capacitor://localhost:8000/home").json()
assert response["sessionRecording"] == make_session_recording_config(
assert response["sessionRecording"] == make_session_recording_decide_response(
{"eventTriggers": ["$pageview", "$exception"]}
)

Expand Down Expand Up @@ -416,6 +421,85 @@ def test_session_replay_config(self, *args):
self.assertEqual(response["sessionRecording"]["canvasFps"], 3)
self.assertEqual(response["sessionRecording"]["canvasQuality"], "0.4")

@parameterized.expand(
[
[
"defaults to none",
None,
None,
{"scriptConfig": None},
False,
],
[
"must have allowlist",
"new-recorder",
None,
{"scriptConfig": None},
False,
],
[
"ignores empty allowlist",
"new-recorder",
[],
{"scriptConfig": None},
False,
],
[
"wild card works",
"new-recorder",
["*"],
{"scriptConfig": {"script": "new-recorder"}},
False,
],
[
"can have wild card and team id",
"new-recorder",
["*"],
{"scriptConfig": {"script": "new-recorder"}},
True,
],
[
"allow list can exclude",
"new-recorder",
["9999", "9998"],
{"scriptConfig": None},
False,
],
[
"allow list can include",
"new-recorder",
["9999", "9998"],
{"scriptConfig": {"script": "new-recorder"}},
True,
],
]
)
def test_session_recording_script_config(
self,
_mock_is_connected: Mock,
_name: str,
rrweb_script_name: str | None,
team_allow_list: list[str] | None,
expected: dict,
include_team_in_allowlist: bool,
) -> None:
self._update_team(
{
"session_recording_opt_in": True,
}
)

if team_allow_list and include_team_in_allowlist:
team_allow_list.append(f"{self.team.id}")

with self.settings(
SESSION_REPLAY_RRWEB_SCRIPT=rrweb_script_name,
SESSION_REPLAY_RRWEB_SCRIPT_ALLOWED_TEAMS=",".join(team_allow_list or []),
):
response = self._post_decide(api_version=3)
assert response.status_code == 200
assert response.json()["sessionRecording"] == make_session_recording_decide_response(expected)

def test_exception_autocapture_opt_in(self, *args):
# :TRICKY: Test for regression around caching
response = self._post_decide().json()
Expand Down Expand Up @@ -473,7 +557,7 @@ def test_user_session_recording_opt_in_wildcard_domain(self, *args):
)

response = self._post_decide(origin="https://random.example.com").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()
self.assertEqual(response["supportedCompression"], ["gzip", "gzip-js"])

# Make sure the domain matches exactly
Expand All @@ -492,7 +576,7 @@ def test_user_session_recording_evil_site(self, *args):
assert response["sessionRecording"] is False

response = self._post_decide(origin="https://example.com").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

def test_user_autocapture_opt_out(self, *args):
# :TRICKY: Test for regression around caching
Expand Down Expand Up @@ -528,19 +612,19 @@ def test_user_session_recording_allowed_when_no_permitted_domains_are_set(self,
self._update_team({"session_recording_opt_in": True, "recording_domains": []})

response = self._post_decide(origin="any.site.com").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

def test_user_session_recording_allowed_for_android(self, *args) -> None:
self._update_team({"session_recording_opt_in": True, "recording_domains": ["https://my-website.io"]})

response = self._post_decide(origin="any.site.com", user_agent="posthog-android/3.1.0").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

def test_user_session_recording_allowed_for_ios(self, *args) -> None:
self._update_team({"session_recording_opt_in": True, "recording_domains": ["https://my-website.io"]})

response = self._post_decide(origin="any.site.com", user_agent="posthog-ios/3.1.0").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

def test_user_session_recording_allowed_when_permitted_domains_are_not_http_based(self, *args):
self._update_team(
Expand All @@ -551,7 +635,7 @@ def test_user_session_recording_allowed_when_permitted_domains_are_not_http_base
)

response = self._post_decide(origin="capacitor://localhost:8000/home").json()
assert response["sessionRecording"] == make_session_recording_config()
assert response["sessionRecording"] == make_session_recording_decide_response()

@snapshot_postgres_queries
def test_web_app_queries(self, *args):
Expand Down Expand Up @@ -2906,10 +2990,13 @@ def test_decide_doesnt_error_out_when_database_is_down(self, *args):

response = self._post_decide(api_version=2, origin="https://random.example.com").json()

assert response["sessionRecording"] == make_session_recording_config(
{
"sampleRate": "0.20",
}
self.assertEqual(
response["sessionRecording"],
make_session_recording_decide_response(
{
"sampleRate": "0.20",
}
),
)

self.assertEqual(response["supportedCompression"], ["gzip", "gzip-js"])
Expand All @@ -2928,10 +3015,13 @@ def test_decide_doesnt_error_out_when_database_is_down(self, *args):
with connection.execute_wrapper(QueryTimeoutWrapper()):
response = self._post_decide(api_version=2, origin="https://random.example.com").json()

assert response["sessionRecording"] == make_session_recording_config(
{
"sampleRate": "0.20",
}
self.assertEqual(
response["sessionRecording"],
make_session_recording_decide_response(
{
"sampleRate": "0.20",
}
),
)

self.assertEqual(response["supportedCompression"], ["gzip", "gzip-js"])
Expand Down Expand Up @@ -3730,8 +3820,14 @@ def test_decide_doesnt_error_out_when_database_is_down_and_database_check_isnt_c
response = self._post_decide(api_version=3, origin="https://random.example.com").json()
response = self._post_decide(api_version=3, origin="https://random.example.com").json()

assert response["sessionRecording"] == make_session_recording_config({"sampleRate": "0.40"})

self.assertEqual(
response["sessionRecording"],
make_session_recording_decide_response(
{
"sampleRate": "0.40",
}
),
)
self.assertEqual(response["supportedCompression"], ["gzip", "gzip-js"])
self.assertEqual(response["siteApps"], [])
self.assertEqual(
Expand Down
Loading
Loading