feat: limit posthog recorder by max team #30764
Conversation
| @@ -517,6 +517,24 @@ def _session_recording_domain_not_allowed(team: Team, request: HttpRequest) -> b | |||
| return team.recording_domains and not on_permitted_recording_domain(team.recording_domains, request) | |||
|
|
|||
|
|
|||
| # KLUDGE: this is duplicated in posthog/models/remote_config.py | |||
There was a problem hiding this comment.
rather than collapse this duplication that confused me, i've at least marked it for the future traveller
There was a problem hiding this comment.
One more word to google.
But if it's exactly the same code - why not to move it to some sort of utils? Again, I assume there's a specific reason for that, I just want to know it :)
There was a problem hiding this comment.
oh, makes sense then. would still be a bit happier to have it reused instead of the comment (as it suggests the change later anyway) 😄
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR updates session replay configuration to use a maximum team threshold for custom rrweb scripts, replacing the earlier allow list.
- Updated
/posthog/settings/session_replay.pyto useSESSION_REPLAY_RRWEB_SCRIPT_MAX_ALLOWED_TEAMSas a maximum team threshold. - Introduced
_should_have_custom_rrweb_scriptin/posthog/models/remote_config.pyto centralize the team ID threshold logic. - Refactored tests in
/posthog/api/test/test_decide.pyto reflect new threshold-based logic. - Modified
/posthog/api/decide.pyto leverage centralized custom script logic. - Note: Ensure consistent type conversion for team ID comparisons to avoid potential pitfalls.
4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| # can be a comma separated list of team ids or '*' to allow all teams | ||
| SESSION_REPLAY_RRWEB_SCRIPT_ALLOWED_TEAMS = get_list(get_from_env("SESSION_REPLAY_RRWEB_SCRIPT_ALLOWED_TEAMS", "")) | ||
| # can be * for all teams or a number to limit to any team with an id less than the number | ||
| SESSION_REPLAY_RRWEB_SCRIPT_MAX_ALLOWED_TEAMS = get_from_env("SESSION_REPLAY_RRWEB_SCRIPT_MAX_ALLOWED_TEAMS", "-1") |
There was a problem hiding this comment.
i went for above and below a fixed team number
we could do something like the session id hashing in posthog-js for a repeatable above or below a sample rate but this felt simpler
pauldambra
requested review from
a team,
sortafreel and
veryayskiy
and removed request for
a team
| @@ -517,6 +517,24 @@ def _session_recording_domain_not_allowed(team: Team, request: HttpRequest) -> b | |||
| return team.recording_domains and not on_permitted_recording_domain(team.recording_domains, request) | |||
|
|
|||
|
|
|||
| # KLUDGE: this is duplicated in posthog/models/remote_config.py | |||
There was a problem hiding this comment.
One more word to google.
But if it's exactly the same code - why not to move it to some sort of utils? Again, I assume there's a specific reason for that, I just want to know it :)
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
Now that we've tested this with production traffic we can limit by max team instead of specific teams so that we can slowly roll it out