Skip to content
This repository has been archived by the owner on Jan 2, 2024. It is now read-only.

FR: nop when reporting usage for known, but non-metered feature #206

Open
isaacs opened this issue Dec 21, 2022 · 3 comments
Open

FR: nop when reporting usage for known, but non-metered feature #206

isaacs opened this issue Dec 21, 2022 · 3 comments
Assignees
Labels
fr Feature request

Comments

@isaacs
Copy link
Contributor

isaacs commented Dec 21, 2022

What are you trying to do?

If you push a model like this:

{
  "plans": {
    "plan:x@1": {
      "title": "plan:x@1",
      "features": {
        "feature:x": {
          "title": "only this"
        }
      }
    }
  }
}

then the result is a feature that's not reportable, as if it had "base": 0 set on the feature.

It'd be more explicit/clear if tier pull would present it this way, so every feature has exactly one of either base or tiers.

How should we solve this?

Add "base": 0 in tier pull if the feature definition doesn't have tiers or a base price.

What is the impact of not solving this?

Not much, really. Just means that it's a little less explicit/clear what kind of feature is created by default. Ie, should "feature:x": {} be treated as {"base":0} or {"tiers":[{}]}? (Both are unlimited and free, but only the second is valid for tier report.)

Anything else?

It might be worth considering treating "feature:x":{} as {"tiers":[{}]} instead of {"base":0}, just to avoid the footgun of creating an unlimited feature that isn't a valid argument for tier report.

Additionally, it'd be nice if /v1/limits reported somehow whether a feature is metered or not, so that the SDK can prevent someone from using tier.report(). As it is now, a {"base":n} feature returns usage: [ { feature: 'feature:x', used: 1, limit: 9223372036854776000 } ] from the /v1/limits call, so it looks like tier.report() should work fine.

Alternatively, perhaps tier.report() should just no-op if a feature isn't metered (since usage is unlimited anyway?) but that feels like it might open the door for some confusing behavior.

@isaacs isaacs added the fr Feature request label Dec 21, 2022
@isaacs
Copy link
Contributor Author

isaacs commented Dec 21, 2022

Oh, also it seems that if you push "feature:x":{"base":0} then tier pull shows it as "feature:x":{}, so at least it's consistent as-is, if not as explicit as it could be.

@bmizerany bmizerany self-assigned this Jan 26, 2023
@bmizerany
Copy link
Contributor

It might be worth considering treating "feature:x":{} as {"tiers":[{}]} instead of {"base":0}, just to avoid the footgun of creating an unlimited feature that isn't a valid argument for tier report.

Features are licensed by default. Stripe requires at least a single tier for metered prices, so we would have had to add extra meta to the price to say, "this is really metered despite it being stored in Stripe as licensed." Rather than fight Stripe, I leaned into its defaults.

Additionally, it'd be nice if /v1/limits reported somehow whether a feature is metered or not, so that the SDK can prevent someone from using tier.report(). As it is now, a {"base":n} feature returns usage: [ { feature: 'feature:x', used: 1, limit: 9223372036854776000 } ] from the /v1/limits call, so it looks like tier.report() should work fine.

Alternatively, perhaps tier.report() should just no-op if a feature isn't metered (since usage is unlimited anyway?) but that feels like it might open the door for some confusing behavior.

It would be even better if the SDK didn't have to know if something is metered; instead, we accept, log, and move on. This allows users to track usage without billing it. Reporting usage for a feature that exists but is not "reportable" should be a nop. I should be clear if we only nop for known features and report an error if we see an unknown feature.

I'm changing this ticket to track work on "nop if reporting usage for known, but non-metered feature."

@bmizerany bmizerany changed the title FR: explicitly treat feature without base or tiers as "base":0 FR: nop when reporting usage for known, but non-metered feature Feb 23, 2023
@isaacs
Copy link
Contributor Author

isaacs commented Feb 24, 2023

It would be even better if the SDK didn't have to know if something is metered; instead, we accept, log, and move on.

Yeah, I kinda threw that in there as a "well obviously this is a bad idea" thought, but now that I consider it again, I agree, /v1/report should just accept a non-metered feature and nop it.

That also avoids the footgun of having a feature that's metered in one plan and flat rate in another.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fr Feature request
Projects
None yet
Development

No branches or pull requests

2 participants