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: Feature gate session replay controls using available_product_features #19401

Merged
merged 15 commits into from
Jan 2, 2024

Conversation

xrdt
Copy link
Contributor

@xrdt xrdt commented Dec 19, 2023

Problem

Need to only allow new users to have access to these controls.

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

@@ -41,7 +41,7 @@ import { NodeKind } from './queries/schema'

export type Optional<T, K extends string | number | symbol> = Omit<T, K> & { [K in keyof T]?: T[K] }

// Keep this in sync with backend constants (constants.py)
// Keep this in sync with backend constants/features/{product_name}.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feels like another thing we could automatically publish from billing so we don't have to manually keep it in sync.

Copy link
Contributor

github-actions bot commented Dec 19, 2023

Size Change: 0 B

Total Size: 2 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 2 MB

compressed-size-action

Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

Perhaps I've missed something but this doesn't feel like something we should control with billing features...

Why would we only offer it if you are subscribed? Even on a free tier it makes a lot of sense and keeps people using it. If you are thinking of upgrading but are worried about cost, maybe the thing you would do first is add the controls before upgrading.
Similarly if you downgrade but had some testings configured, you're now stuck with whatever setting you had...

Unless there is really strong reasoning why we would do this, I would personally vote to keep it as a flagged feature and gradually roll out to everyone. If we want to limit to only new users - feature flag, keeps it simple and allows us to conditionally add more legacy users for example as a tool for customer success.

@raquelmsmith
Copy link
Member

raquelmsmith commented Dec 20, 2023

@benjackwhite with the new plans it won't only be offered when you are sub'd. It'll be available on the free plans too. (you can peek the plans here https://github.com/PostHog/billing/pull/437/files)

BUT because we want to pair the rollout of more sensible pricing with the rollout of the controls (ie. you get these controls, but you also have a lower free tier to accommodate that), that means that existing plans (with the higher free tier) won't get access to the controls.

We know what plans they are on from billing, so adding the feature gating via billing makes a lot of sense.

Once we see how much people use these and what the impacts are we will consider migrating everyone to the new plans - until then if people want them they can be put on the new plans manually.

@xrdt
Copy link
Contributor Author

xrdt commented Dec 21, 2023

You can now control whether the settings show either with the feature flag or by setting individual settings:

Screen.Recording.2023-12-21.at.1.33.46.PM.mp4

@xrdt xrdt requested a review from raquelmsmith December 22, 2023 17:05
@raquelmsmith
Copy link
Member

This can go in now, no need to wait for billing launch since we are still looking for the flag value.

@xrdt xrdt requested a review from benjackwhite December 22, 2023 18:19
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

  • chromium: 0 added, 4 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

Happy to defer to your judgement. Still feels weird that we would need both available features and feature flag but if it works, it works.

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

There aren't many people with the flag on, so we can follow up by checking price impact of moving them to the new plans to get to a place where we can remove the flag but there's no rush for that

@xrdt xrdt merged commit b8b90c1 into master Jan 2, 2024
79 checks passed
@xrdt xrdt deleted the by/feature-gate-session-replay-product-features branch January 2, 2024 19:37
fuziontech pushed a commit that referenced this pull request Jan 4, 2024
…atures (#19401)

* feature gate session replay control using available_product_features

* separate out feature checks

* refine the separate feature checks

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: Bianca Yang <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
fuziontech added a commit that referenced this pull request Jan 4, 2024
* master: (94 commits)
  feat: Add cta on billing for >20k annual spend customers (#19508)
  refactor(temporal/squash): Support flat person override table in squash workflow (#19347)
  fix(surveys): remove link in user interview template (#19584)
  feat: populate plugin capabilities on install and edit (#19188)
  chore: Make plugin-server ignore deleted plugin configs (#18444)
  chore: Add Flutter feature flags snippets (#19563)
  fix(bi): fixed some of the query duplications (#19573)
  fix: assume typeless series nodes are of type events node (#19550)
  chore(deps): bump chromaui/action from 1 to 10 (#19560)
  fix(trends): fix breakdowns persons label (#19534)
  fix(trends): fix breakdown legend (#19533)
  feat: incremental updates for mobile transformer (#19567)
  chore(deps): bump peter-evans/find-comment from 1 to 2 (#19559)
  chore(data-warehouse): cleanup unused celery code and extend time (#19568)
  Revert "feat(data-warehouse): hubspot integration" (#19569)
  feat(data-warehouse): hubspot integration (#19529)
  feat: Feature gate session replay controls using available_product_features (#19401)
  fix: Padding on bullet lists (#19565)
  chore: Post 3000 LemonButtton cleanup (#19540)
  chore(deps): bump aws-actions/amazon-ecr-login from 1 to 2 (#19558)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants