posthog icon indicating copy to clipboard operation
posthog copied to clipboard

feat: limit grace period for annual customers to 7 days

Open pawel-cebula opened this issue 7 months ago • 5 comments

Problem

Annual customers all have trust score of 15, which resulted in them never getting quota limited, which left a loophole to allow having unlimited free usage and/or over-ingestion in case of spikes.

Changes

Introduces a 7 day grace period for customers with trust score of 15

  • We can continue using never_drop_data flag for VIPs or temporary exceptions
  • These changes will be combined with setting up alerts for sales team about customers where we're ignoring quota limiting due to trust score of 15 or never_drop_data so they can get in touch with them in parallel

For more context, see: https://posthog.slack.com/archives/C08ERRQT41E/p1748538038187629

Did you write or update any docs for this change?

  • [x] No docs needed for this change

How did you test this code?

Updated tests

pawel-cebula avatar Jun 04 '25 11:06 pawel-cebula

Approved, but I think the fact that QUOTA_LIMIT_... constants and GRACE_PERIOD_DAYS values are not aligned is something we should revisit in the future.

Can you clarify what you mean by the values not being aligned?

pawel-cebula avatar Jun 04 '25 12:06 pawel-cebula

Not saying it's wrong, just that is confusing at first sight that this is how it looks after being parsed:

GRACE_PERIOD_DAYS: dict[int, int] = {
    3: 0,
    7: 1,
    10: 3,
    15: 7,
}

It makes me think the trust score could be the same as the grace period days, but for some reason they aren't.

patricio-posthog avatar Jun 04 '25 12:06 patricio-posthog

Ah got it, yeah I understand it might appear confusing. They shouldn't need to be aligned though IMO. My understanding is that trust scores (like membership types) are ints that are spread out so that we can in the future add more granular ones in between. They can serve many different purposes, related to other things, not just grace period days. The grace periods can also change over the time.

Maybe a way to reduce similar confusion would be to turn trust scores into enums (and rename it to map)?

This way it would look like:

GRACE_PERIOD_DAYS_MAP: dict[int, int] = {
    CustomerTrustScore.LOW: QUOTA_LIMIT_LOW_TRUST_GRACE_PERIOD_DAYS,
    CustomerTrustScore.MEDIUM: QUOTA_LIMIT_MEDIUM_TRUST_GRACE_PERIOD_DAYS,
    CustomerTrustScore.MEDIUM_HIGH: QUOTA_LIMIT_MEDIUM_HIGH_TRUST_GRACE_PERIOD_DAYS,
}

(will keep it out of scope here since it's used in a lot more places)

pawel-cebula avatar Jun 04 '25 12:06 pawel-cebula

Yeah I was going to suggest that, we shouldn't use integers, specially 1, 7, 15, that seem to have some kind of meaning, if they don't. We should replace them with enums. But nothing urgent.

patricio-posthog avatar Jun 04 '25 12:06 patricio-posthog

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. If you want to permanentely keep it open, use the waiting label.

posthog-bot avatar Jun 13 '25 07:06 posthog-bot

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. If you want to permanentely keep it open, use the waiting label.

posthog-bot avatar Jun 23 '25 07:06 posthog-bot

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. If you want to permanentely keep it open, use the waiting label.

posthog-bot avatar Jul 01 '25 07:07 posthog-bot

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. If you want to permanentely keep it open, use the waiting label.

posthog-bot avatar Jul 09 '25 07:07 posthog-bot

Changed from 7 to 5 days (Simon felt even a day would be sufficient if they've explicitly set limits but let's start more generous and adjust later if it still makes sense)

pawel-cebula avatar Jul 15 '25 10:07 pawel-cebula