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: signal which products are quota limited #16777

Closed
wants to merge 14 commits into from

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Jul 26, 2023

Problem

In #16725 I changed capture so that we could have posthog-js clients not continue sending session recordings when they were quota limited

But, that meant all older clients would retry traffic that they previously wouldn't have done.

We had to revert that, but still need to reduce load from recordings clients

Changes

  • Still return a 200 when dropping events due to quota limiting
  • If recordings were quota limited then return a list of limited products in that request as part of the payload

So, the possible responses now are

{"status": 1}

and

{"status": 1, "quotaLimited": ["recordings"]}

I considered more structure in the payload

e.g.

{
    "status": 1,
    "quota_limited": {
        "recordings": {
             "retry-after": 60
        }
    }
}

That would mean we have space to amend this in the future, but this is the first change to response shape in 2 or 3 years, so, it's reasonable to assume that it won't change much from here.

How did you test this code?

developer tests

@pauldambra pauldambra requested a review from benjackwhite July 26, 2023 14:16
@pauldambra pauldambra requested a review from benjackwhite July 27, 2023 16:10
@pauldambra
Copy link
Member Author

pairs with PostHog/posthog-js#765

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.

200 lgtm

@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.

@posthog-bot posthog-bot removed the stale label Aug 9, 2023
@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.

@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@posthog-bot posthog-bot closed this Sep 1, 2023
@pauldambra pauldambra reopened this Sep 13, 2023
@posthog-bot posthog-bot removed the stale label Sep 14, 2023
@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@pauldambra pauldambra reopened this Oct 18, 2023
@posthog-bot posthog-bot removed the stale label Oct 19, 2023
@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@posthog-bot posthog-bot closed this Nov 3, 2023
@pauldambra pauldambra reopened this Nov 3, 2023
@posthog-bot posthog-bot removed the stale label Nov 6, 2023
@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@pauldambra pauldambra reopened this Dec 13, 2023
@pauldambra pauldambra removed the stale label Dec 13, 2023
@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@pauldambra pauldambra reopened this Feb 12, 2024
@posthog-bot posthog-bot removed the stale label Feb 13, 2024
@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.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

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.

3 participants