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: support usage based pricing #1787

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: support usage based pricing #1787

wants to merge 2 commits into from

Conversation

turip
Copy link
Contributor

@turip turip commented Nov 5, 2024

Overview

Add line-splitting capability to support mid-period invoicing. See README.md for more detailed information.

Algorithm

To achieve the behavior described above, we are using line splitting. By default we would have one line per billing period that would eventually be part of an invoice:

                period.start                                              period.end
Line1 [status=valid] |--------------------------------------------------------|

When the usage-based line can be billed mid-period, we split the line into two:

                     period.start              asOf                              period.end
Line1 [status=split]         |--------------------------------------------------------|
SplitLine1 [status=valid]    |------------------|
SplitLine2 [status=valid]                       |-------------------------------------|

As visible:

  • Line1's status changes from valid to split: it will be ignored in any calculation, it becomes a grouping line between invoices
  • SplitLine1 is created with a period between period.start and asof (time of invoicing): it will be addedd to the freshly created invoice
  • SplitLine2 is created with a period between asof and period.end: it will be pushed to the gathering invoice

When creating a new invoice between asof and period.end the same logic continues, but without marking SplitLine2 split, instead the new line is added to the original line's parent line:

                     period.start              asOf1          asof2                period.end
Line1 [status=split]         |--------------------------------------------------------|
SplitLine1 [status=valid]    |------------------|
SplitLine2 [status=valid]                       |---------------|
SplitLine3 [status=valid]                                       |---------------------|

This flattening approach allows us to not to have to recusively traverse lines in the database.

@turip turip force-pushed the feat/ubp branch 4 times, most recently from 8b23ff1 to dc37609 Compare November 6, 2024 10:16
@turip turip force-pushed the feat/ubp branch 4 times, most recently from de2df86 to f38a546 Compare November 7, 2024 19:59
@turip turip added release-note/feature Release note: Exciting New Features area/billing labels Nov 7, 2024
@turip turip requested a review from hekike November 7, 2024 20:00
@turip turip force-pushed the feat/ubp branch 2 times, most recently from 2c527ea to e85d325 Compare November 7, 2024 20:43
@turip turip marked this pull request as ready for review November 7, 2024 20:43
import "time"

const (
DefaultMeterResolution = time.Minute
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be removed as soon as we have better granularity.

openmeter/billing/README.md Outdated Show resolved Hide resolved
openmeter/billing/adapter/invoice.go Outdated Show resolved Hide resolved
postSplitAtLine := l.CloneForCreate(UpdateInput{
ParentLine: &parentLine,
Status: billingentity.InvoiceLineStatusValid,
PeriodStart: splitAt,
Copy link
Contributor

Choose a reason for hiding this comment

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

just double checking: is this is also splitAt and and previous ends at splitAt one should be exclusive when we query usage

Copy link
Contributor Author

@turip turip Nov 8, 2024

Choose a reason for hiding this comment

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

The code assumes that (added the comment):

// Period represents a time period, in billing the time period is always interpreted as
// [start, end) (i.e. start is inclusive, end is exclusive).

This is aligned with the current logic where clickhouse's tumbleStart is inclusive and tumbleEnd is exclusive.


switch meter.Aggregation {
case models.MeterAggregationSum, models.MeterAggregationCount,
models.MeterAggregationMax, models.MeterAggregationUniqueCount:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure Max works here. Max of period chunk is different then max of whole period

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry, I was in a bit of rush for the meters, I have updated the docs and the algorithm.

Aggregation: in.Meter.Aggregation,
FilterSubject: in.Subjects,
From: &in.Line.Period.Start,
To: &in.Line.Period.End,
Copy link
Contributor

Choose a reason for hiding this comment

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

feature has group by filters that should be applied here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- refactor diff calculation
- use jsonb for usage attribution
- fix README.md typos
- document period
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/billing release-note/feature Release note: Exciting New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants