cloud billing customer creation and reporting #5050

pjain1 · 2024-06-10T20:16:22Z

This includes - Creating a customer in external billing system on new org creation and assigning a billing plan to it. Plan metadata configured in the billing system should have quotas that will be fetched to be enforced as well reportable metric names. These metrics name will directly corresponds to the apis in the metrics project to fetch the corresponding usage. Usage reporter will use the apis to report usage to the billing system.

Next -

Enforce usage limits especially managed data bytes, will go into reconciler
Create metrics APIs in metrics project that will be used to fetch usage
APIs for UI to get list of public plans, current subscriptions for a customer and change plan for a customer
Rill cli cmds for the same
Handling of trail period and subscription end in web-admin, APIs for this, will need product input

admin/database/database.go

begelundmuller · 2024-06-11T15:56:14Z

admin/database/database.go

@@ -238,6 +239,9 @@ type Organization struct {
 	QuotaSlotsTotal         int       `db:"quota_slots_total"`
 	QuotaSlotsPerDeployment int       `db:"quota_slots_per_deployment"`
 	QuotaOutstandingInvites int       `db:"quota_outstanding_invites"`
+	QuotaNumUsers           int       `db:"quota_num_users"`
+	QuotaManagedDataBytes   int64     `db:"quota_managed_data_bytes"`
+	BillingCustomerID       *string   `db:"billing_customer_id"` // review: should this be a struct to store more metadata


I'm fine with both approaches, but yeah if we need to add some billing provider-specific fields, it might be easier to just have a JSON payload called something like billing_properties.

begelundmuller · 2024-06-11T15:59:35Z

admin/database/database.go

+	Annotations            map[string]string `db:"annotations"`
+	CreatedOn              time.Time         `db:"created_on"`
+	UpdatedOn              time.Time         `db:"updated_on"`
+	NextUsageReportingTime time.Time         `db:"next_usage_reporting_time"`


Could we make this UsageReportedOn? Then a null default value would be meaningful, and the caller can add the delta for next report time at query time. (Also using "on" for consistency with CreatedOn and UpdatedOn.)

UsageReportedOn seems like the time on which last usage was reported but In the code this is used as the reporting start time for next reporting window. This window may be bigger than one delta as reporting may be delayed because of data unavailability or other number of reasons. Do you still think this makes sense, so I will make the change

seems like the time on which last usage was reported

Yes, so what I meant was that I think the code will be cleaner if it tracks the last reporting time. That leaves the logic of deciding a next time only up to the worker – whereas NextUsageReportingTime creates coupled logic between the creator of the project and the reporting worker.

admin/database/database.go

admin/billing/orb.go

admin/billing/biller.go

begelundmuller · 2024-06-12T10:01:00Z

admin/worker/billing_reporter.go

@@ -0,0 +1,247 @@
+package worker


I think it's worth going over this file once more with an aim of simplifying the code where possible. There are some places with pretty deep nesting and if/else scenarios.

Also, have you considered syncing events in bulk for all orgs/projecst in one query (going over all events in the bucket) instead of querying and uploading per org/project? It seems the Orb APIs would support that and could reduce the latency and number of queries by a lot.

Yeah its pretty busy, not happy with it. Tried simplifying a bit by breaking logic into functions to handle org and then its projects. Also changed reporting logic to first collect usage for all projects in an org and reporting all at once. Two more places to look into -

Reporting usage to Orb for all orgs, not sure if we would hit issues with big payload

Usage availability is fetched for all projects of an org in one go from rill-cloud-metrics but fetching usage is still done per project per bucket per metric. Each project might have different buckets to report and metrics to report may change as per plan so creating a single api to fetch usage per plan can be hard to keep in sync.

What do you think, may be handling too many corner cases.

Each project might have different buckets to report and metrics to report may change as per plan so creating a single api to fetch usage per plan can be hard to keep in sync.

My hope would be that we could simplify usage reporting a lot by not trying to do such a granular breakdown. It should also lead to fewer, more efficient queries to the DB. Unless Orb charges a lot of money for redundant metrics, it seems simpler to just ship all non-zero billable metrics to them and have the logic of which metrics to use implemented there.

Of course we would probably need to batch the uploads to Orb to not upload huge payloads.

I'm imagining a query like this on our side (pseudo-code):

SELECT instance_id, <date trunc> as event_time, metric_name, <agg> as value FROM metrics WHERE event_time > <from time> AND event_time < <to time> GROUP BY ALL ORDER BY

Then in the worker, it just does a lookup and caches each instance_id, enriches the events with info about the instance's org, and does a bulk upload to Orb of e.g. every 1k events.

…view comments

admin/billing/biller.go

admin/billing/orb.go

admin/database/database.go

begelundmuller · 2024-06-21T11:33:09Z

admin/worker/billing_reporter.go

@@ -0,0 +1,247 @@
+package worker


Each project might have different buckets to report and metrics to report may change as per plan so creating a single api to fetch usage per plan can be hard to keep in sync.

My hope would be that we could simplify usage reporting a lot by not trying to do such a granular breakdown. It should also lead to fewer, more efficient queries to the DB. Unless Orb charges a lot of money for redundant metrics, it seems simpler to just ship all non-zero billable metrics to them and have the logic of which metrics to use implemented there.

Of course we would probably need to batch the uploads to Orb to not upload huge payloads.

I'm imagining a query like this on our side (pseudo-code):

SELECT instance_id, <date trunc> as event_time, metric_name, <agg> as value FROM metrics WHERE event_time > <from time> AND event_time < <to time> GROUP BY ALL ORDER BY

Then in the worker, it just does a lookup and caches each instance_id, enriches the events with info about the instance's org, and does a bulk upload to Orb of e.g. every 1k events.

admin/billing/biller.go

admin/billing/orb.go

begelundmuller · 2024-06-24T18:15:58Z

admin/billing/orb.go

+	for i := 0; i < len(plans.Data); i++ {
+		billingPlan, err := getBillingPlanFromOrbPlan(&plans.Data[i])
+		if err != nil {
+			return nil, err
+		}
+		billingPlans = append(billingPlans, billingPlan)
+	}


Should we maybe filter out those plans where Name == ""? In case we forget to set it in Orb, then could someone send a manual API call to upgrade to "" and then get access to some internal plan? Or is there something that protects against that?

Good catch, did not thought about it but it cannot happen currently, in the API where GetPlanByName is used, a check exists to disallow empty string. Also there is option to add plan in the cli cmd using biller plan id as well (not sure if we need that), so filtering would cause that to fail.

But to be more defensive I can return ErrNotFound from GetPlanByName if name is empty ?

Sounds good with the additional defensive option

admin/metrics/client.go

admin/worker/billing_reporter.go

admin/database/database.go

admin/worker/billing_reporter.go

proto/rill/admin/v1/api.proto

begelundmuller

Also check out the merge conflicts

proto/rill/admin/v1/api.proto

begelundmuller · 2024-06-28T12:33:47Z

proto/rill/admin/v1/api.proto

+  // DeleteOrganizationSubscription deletes the given subscription for the organization
+  rpc DeleteOrganizationBillingSubscription(DeleteOrganizationBillingSubscriptionRequest) returns (DeleteOrganizationBillingSubscriptionResponse) {
+    option (google.api.http) = {delete: "/v1/organizations/{org_name}/billing/subscriptions/{subscription_id}"};
+  }


Maybe we already discussed this – but why is it we need the ability to delete a subscription? Won't it put the org into a weird state? If you want to stop a paid subscription, shouldn't you just call UpdateOrganizationBillingPlan to change to a free plan?

Yeah my original intention was to allow for fix for issues that could arise from manual assignment, also don't see any free plan in the mocks, I just see Starter plan which has 30 days trial period and then its all paid. In that case as well if no subscprition then org should be hibernated, not sure but its a separate issue. Anyways removed.

begelundmuller · 2024-06-28T12:34:53Z

proto/rill/admin/v1/api.proto

+  optional string plan_name = 2;
+  optional string biller_plan_id = 3;


Can we decide on just one way of doing this?

moved to using plan name only as its a more friendly option for cli but also enforces that all plans defined in Orb have an external plan id defined.

pjain1 added 2 commits June 11, 2024 01:44

cloud billing customer creation and reporting

5226b4f

Merge branch 'main' into cloud_billing

cd7f42e

pjain1 marked this pull request as draft June 10, 2024 20:42

pjain1 added 4 commits June 11, 2024 02:14

col name

3b2a816

fix tx ctx

61dfade

customer if refactor

53749ab

sudo api for assigning changing billing plan

eb4e9ce

begelundmuller requested changes Jun 12, 2024

View reviewed changes

pjain1 added 5 commits June 12, 2024 23:42

apis for listing plans, subscriptions, changing plan, resolve some re…

038a4f0

…view comments

rill cloud metrics api

2ff4b24

reporter refactor, use org ids, other review comments

c39ba48

fix org insert

d7f5a1e

billing related cli cmds

1b1a5f1

pjain1 requested a review from begelundmuller June 18, 2024 07:02

pjain1 added 6 commits June 19, 2024 15:27

fix issues

3fb33c1

return early in case of noop billing

b2e2ac8

lint fixes

8d175a5

fixes

037b595

safety fixes

ea471ba

Merge branch 'main' into cloud_billing

a5fb919

pjain1 marked this pull request as ready for review June 20, 2024 11:01

begelundmuller requested changes Jun 21, 2024

View reviewed changes

pjain1 added 2 commits June 24, 2024 18:02

simplify reporting logic, review comments

04b387b

batch reporting

c761d39

pjain1 requested a review from begelundmuller June 24, 2024 12:55

pjain1 added 4 commits June 24, 2024 18:26

fix crontab

74e8505

add validation

c034a40

fix push

f50dacc

Merge branch 'main' into cloud_billing

5cee68d

begelundmuller requested changes Jun 24, 2024

View reviewed changes

pjain1 added 5 commits June 25, 2024 13:11

review comments

17cd5d2

Merge branch 'main' into cloud_billing

84c8d21

pagination for getting usage data

02fe010

assume single subscription

3fee383

remove unecessary change

39a3e93

begelundmuller reviewed Jun 28, 2024

View reviewed changes

pjain1 added 2 commits June 28, 2024 22:05

review comments

4d27a70

Merge branch 'main' into cloud_billing

4f27c74

pjain1 requested a review from begelundmuller June 28, 2024 16:45

begelundmuller approved these changes Jun 28, 2024

View reviewed changes

pjain1 merged commit 9c87336 into main Jul 1, 2024
7 checks passed

pjain1 deleted the cloud_billing branch July 1, 2024 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cloud billing customer creation and reporting #5050

cloud billing customer creation and reporting #5050

pjain1 commented Jun 10, 2024 •

edited

Loading

begelundmuller Jun 11, 2024

begelundmuller Jun 11, 2024

pjain1 Jun 14, 2024

begelundmuller Jun 21, 2024

begelundmuller Jun 12, 2024

pjain1 Jun 15, 2024 •

edited

Loading

begelundmuller Jun 21, 2024

begelundmuller Jun 21, 2024

begelundmuller Jun 24, 2024

pjain1 Jun 25, 2024 •

edited

Loading

begelundmuller Jun 28, 2024

begelundmuller left a comment

begelundmuller Jun 28, 2024

pjain1 Jun 28, 2024

begelundmuller Jun 28, 2024

pjain1 Jun 28, 2024

		optional string plan_name = 2;
		optional string biller_plan_id = 3;

cloud billing customer creation and reporting #5050

cloud billing customer creation and reporting #5050

Conversation

pjain1 commented Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjain1 Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjain1 Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

begelundmuller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjain1 commented Jun 10, 2024 •

edited

Loading

pjain1 Jun 15, 2024 •

edited

Loading

pjain1 Jun 25, 2024 •

edited

Loading