[usage] Implement `GetBilledUsage` RPC #11387

andrew-farries · 2022-07-14T15:38:10Z

Description

Implement the usage API GetBilledUsage RPC. Given an attribution ID, return usage records corresponding to that ID.

⚠️ No date filtering yet - this just returns all usage records for the attribution ID regardless of age. ⚠️

Related Issue(s)

Part of #9036

How to test

Unit tests for the RPC.

To test the API manually:

Run the usage service locally against the database in the preview environment for this PR:

kubectl port-forward svc/mysql 3306:3306
cd components/usage
DB_HOST=localhost DB_USERNAME=gitpod DB_PASSWORD=<> DB_PORT=3306 go run . run

Use a gRPC client like evans to run against the usage API:

Release Notes

NONE

Werft options:

/werft with-preview

werft-gitpod-dev-com · 2022-07-14T15:39:50Z

started the job as gitpod-build-af-real-data-from-usage-api.11 because the annotations in the pull request description changed
(with .werft/ from main)

andrew-farries · 2022-07-15T06:00:31Z

components/usage/pkg/apiv1/usage.go

+			AttributionId:  string(usageRecord.AttributionID),
+			UserId:         "",
+			TeamId:         "",
+			WorkspaceId:    "",
+			WorkspaceType:  "",
+			ProjectId:      "",
+			InstanceId:     usageRecord.InstanceID.String(),
+			WorkspaceClass: "",
+			StartTime:      timestamppb.New(usageRecord.StartedAt),
+			EndTime:        endTime,
+			Credits:        int64(usageRecord.CreditsUsed),


Regarding the missing fields here, I'm not sure how many we want to store in the usage table ourselves, vs how much we want to obtain by joining with the existing workspace table.

The motivation for storing usage was so that we don't need to recompute it on every query to the usage API, but as long as we have the instanceIds, we should be able to recover the workspace details here with joins.

In general I agree that this is the strategy we should follow. But WorkspaceClass and WorkspaceType should definitely in here, because they are relevant for the billing parts.
And regarding the hierachy-tuple TeamId-ProjectId-UserId-WorkspaceId: I think being able to directly correlate this entry (without the indirection over WorkspaceInstance) is valuable as well, and helps to "place this session on the map" if you will. I think we should have those, too.

FYI We talked in sync, and agreed to wait for @easyCZ and his opinion. 👍

IMO, we should have first-class fields (these are mostly the ones we want to group-by in our views or are important enough to show in list views.

All the fields above meet this first-class definition so I would keem them as is. We should populate them directly from the usage report.

For any extra fields, we should call into WorkspaceInstance for details, or setup a service which pre-joins these for us.

geropl · 2022-07-15T06:05:59Z

@andrew-farries I tested with evans but got an SEGFAULT. I have not clue how to debug that.

Also, wrt to the fields discussion: If we ship less fields then currently defined we should adjust the parsing code here as well.

andrew-farries · 2022-07-15T10:16:18Z

@andrew-farries I tested with evans but got an SEGFAULT. I have not clue how to debug that.

This appears to be a regression between evans v0.10.6 and v0.10.7 released yesterday.

Using v0.10.6 works as expected.

andrew-farries · 2022-07-15T11:47:49Z

PR to add a working version to the dev image here: #11413

andrew-farries · 2022-07-18T07:43:31Z

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-af-real-data-from-usage-api.13
(with .werft/ from main)

andrew-farries · 2022-07-18T07:57:37Z

/hold as this is now based on #11429

easyCZ · 2022-07-18T09:54:48Z

components/usage/pkg/db/workspace_instance_usage.go

@@ -37,3 +38,14 @@ func CreateUsageRecords(ctx context.Context, conn *gorm.DB, records []WorkspaceI

 	return db.CreateInBatches(records, 1000).Error
 }
+
+func GetUsage(ctx context.Context, conn *gorm.DB, attributionId AttributionID) ([]WorkspaceInstanceUsage, error) {


Suggested change

func GetUsage(ctx context.Context, conn *gorm.DB, attributionId AttributionID) ([]WorkspaceInstanceUsage, error) {

func ListUsage(ctx context.Context, conn *gorm.DB, attributionId AttributionID) ([]WorkspaceInstanceUsage, error) {

It returns a collection of records. Get is often used to mean a single record.

easyCZ · 2022-07-18T09:55:50Z

components/usage/pkg/apiv1/usage.go

 )

 var _ v1.UsageServiceServer = (*UsageService)(nil)

 type UsageService struct {
+	conn *gorm.DB
 	v1.UnimplementedUsageServiceServer
 }

 func (us *UsageService) GetBilledUsage(ctx context.Context, in *v1.GetBilledUsageRequest) (*v1.GetBilledUsageResponse, error) {


I didn't spot this before but I'd call the RPC ListUsage because it returns a collection of records

ListUsage or ListBilledUsage?

I guess ListBilledUsage to keep it similar. But generally, a Service should only return one type of Resource (less important for private APIs but for Public ones, it helps with UX). So here, we'd normally have a BilledUsageService, which returns BilledUsage records but no point changing now.

andrew-farries · 2022-07-18T11:51:18Z

/unhold

andrew-farries requested a review from a team July 14, 2022 15:38

roboquat added release-note-none size/L labels Jul 14, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 14, 2022

andrew-farries force-pushed the af/real-data-from-usage-api branch from ad5ad41 to 113617a Compare July 14, 2022 15:39

geropl self-assigned this Jul 15, 2022

andrew-farries commented Jul 15, 2022

View reviewed changes

andrew-farries changed the base branch from main to af/integer-credit-values-in-usage-store July 18, 2022 07:35

andrew-farries force-pushed the af/real-data-from-usage-api branch from 113617a to 3ee2685 Compare July 18, 2022 07:39

roboquat added the do-not-merge/hold label Jul 18, 2022

easyCZ reviewed Jul 18, 2022

View reviewed changes

roboquat added size/XXL and removed size/L labels Jul 18, 2022

easyCZ approved these changes Jul 18, 2022

View reviewed changes

andrew-farries force-pushed the af/real-data-from-usage-api branch 2 times, most recently from 419689b to 7a814fd Compare July 18, 2022 10:31

andrew-farries changed the base branch from af/integer-credit-values-in-usage-store to main July 18, 2022 11:34

andrew-farries force-pushed the af/real-data-from-usage-api branch 2 times, most recently from fed680f to 5156661 Compare July 18, 2022 11:39

[usage] Implement GetBilledUsage rpc

a86efd2

andrew-farries force-pushed the af/real-data-from-usage-api branch from 5156661 to a86efd2 Compare July 18, 2022 11:51

roboquat removed the do-not-merge/hold label Jul 18, 2022

roboquat merged commit e238535 into main Jul 18, 2022

roboquat deleted the af/real-data-from-usage-api branch July 18, 2022 11:57

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[usage] Implement `GetBilledUsage` RPC #11387

[usage] Implement `GetBilledUsage` RPC #11387

andrew-farries commented Jul 14, 2022 •

edited

Loading

werft-gitpod-dev-com bot commented Jul 14, 2022

andrew-farries Jul 15, 2022

geropl Jul 15, 2022

geropl Jul 18, 2022

easyCZ Jul 18, 2022

geropl commented Jul 15, 2022

andrew-farries commented Jul 15, 2022

andrew-farries commented Jul 15, 2022

andrew-farries commented Jul 18, 2022 •

edited by werft-gitpod-dev-com bot

Loading

andrew-farries commented Jul 18, 2022

easyCZ Jul 18, 2022

easyCZ Jul 18, 2022

andrew-farries Jul 18, 2022 •

edited

Loading

easyCZ Jul 18, 2022 •

edited

Loading

andrew-farries commented Jul 18, 2022

	func GetUsage(ctx context.Context, conn *gorm.DB, attributionId AttributionID) ([]WorkspaceInstanceUsage, error) {
	func ListUsage(ctx context.Context, conn *gorm.DB, attributionId AttributionID) ([]WorkspaceInstanceUsage, error) {

[usage] Implement GetBilledUsage RPC #11387

[usage] Implement GetBilledUsage RPC #11387

Conversation

andrew-farries commented Jul 14, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Werft options:

werft-gitpod-dev-com bot commented Jul 14, 2022

andrew-farries Jul 15, 2022

Choose a reason for hiding this comment

geropl Jul 15, 2022

Choose a reason for hiding this comment

geropl Jul 18, 2022

Choose a reason for hiding this comment

easyCZ Jul 18, 2022

Choose a reason for hiding this comment

geropl commented Jul 15, 2022

andrew-farries commented Jul 15, 2022

andrew-farries commented Jul 15, 2022

andrew-farries commented Jul 18, 2022 • edited by werft-gitpod-dev-com bot Loading

andrew-farries commented Jul 18, 2022

easyCZ Jul 18, 2022

Choose a reason for hiding this comment

easyCZ Jul 18, 2022

Choose a reason for hiding this comment

andrew-farries Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

easyCZ Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

andrew-farries commented Jul 18, 2022

[usage] Implement `GetBilledUsage` RPC #11387

[usage] Implement `GetBilledUsage` RPC #11387

andrew-farries commented Jul 14, 2022 •

edited

Loading

andrew-farries commented Jul 18, 2022 •

edited by werft-gitpod-dev-com bot

Loading

andrew-farries Jul 18, 2022 •

edited

Loading

easyCZ Jul 18, 2022 •

edited

Loading