Skip to content

[usage] Persist credits used for each instance in the usage store #11406

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 15, 2022
Merged

[usage] Persist credits used for each instance in the usage store #11406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ab277e
Let the usage reconciler take a pricer
Jul 15, 2022
1f4137b
Extract `CreditsUsedByInstance` method
Jul 15, 2022
0840b7b
Populate credits field for usage records
Jul 15, 2022
11cf3f2
Update tests with new field
Jul 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion components/usage/pkg/controller/billing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ package controller
import (
"context"
"fmt"
"github.com/gitpod-io/gitpod/usage/pkg/stripe"
"math"
"time"

"github.com/gitpod-io/gitpod/usage/pkg/db"
"github.com/gitpod-io/gitpod/usage/pkg/stripe"
)

type BillingController interface {
Expand Down Expand Up @@ -67,6 +69,15 @@ type WorkspacePricer struct {
creditMinutesByWorkspaceClass map[string]float64
}

func (p *WorkspacePricer) CreditsUsedByInstance(instance *db.WorkspaceInstanceForUsage, maxStopTime time.Time) int64 {
runtime := instance.WorkspaceRuntimeSeconds(maxStopTime)
class := defaultWorkspaceClass
if instance.WorkspaceClass != "" {
class = instance.WorkspaceClass
}
return p.Credits(class, runtime)
}

func (p *WorkspacePricer) Credits(workspaceClass string, runtimeInSeconds int64) int64 {
inMinutes := float64(runtimeInSeconds) / 60
return int64(math.Ceil(p.CreditsPerMinuteForClass(workspaceClass) * inMinutes))
Expand Down
23 changes: 12 additions & 11 deletions components/usage/pkg/controller/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,17 @@ func (f ReconcilerFunc) Reconcile() error {
type UsageReconciler struct {
nowFunc func() time.Time
conn *gorm.DB
pricer *WorkspacePricer
billingController BillingController
}

func NewUsageReconciler(conn *gorm.DB, billingController BillingController) *UsageReconciler {
return &UsageReconciler{conn: conn, billingController: billingController, nowFunc: time.Now}
func NewUsageReconciler(conn *gorm.DB, pricer *WorkspacePricer, billingController BillingController) *UsageReconciler {
return &UsageReconciler{
conn: conn,
pricer: pricer,
billingController: billingController,
nowFunc: time.Now,
}
}

type UsageReconcileStatus struct {
Expand Down Expand Up @@ -86,7 +92,7 @@ func (u *UsageReconciler) Reconcile() (err error) {
}
log.Infof("Wrote usage report into %s", filepath.Join(dir, stat.Name()))

err = db.CreateUsageRecords(ctx, u.conn, usageReportToUsageRecords(report))
err = db.CreateUsageRecords(ctx, u.conn, usageReportToUsageRecords(report, u.pricer, u.nowFunc().UTC()))
if err != nil {
return fmt.Errorf("failed to write usage records to database: %s", err)
}
Expand Down Expand Up @@ -136,12 +142,7 @@ func (u UsageReport) CreditSummaryForTeams(pricer *WorkspacePricer, maxStopTime

var credits int64
for _, instance := range instances {
runtime := instance.WorkspaceRuntimeSeconds(maxStopTime)
class := defaultWorkspaceClass
if instance.WorkspaceClass != "" {
class = instance.WorkspaceClass
}
credits += pricer.Credits(class, runtime)
credits += pricer.CreditsUsedByInstance(&instance, maxStopTime)
}

creditsPerTeamID[id] = credits
Expand Down Expand Up @@ -232,7 +233,7 @@ func groupInstancesByAttributionID(instances []db.WorkspaceInstanceForUsage) Usa
return result
}

func usageReportToUsageRecords(report UsageReport) []db.WorkspaceInstanceUsage {
func usageReportToUsageRecords(report UsageReport, pricer *WorkspacePricer, now time.Time) []db.WorkspaceInstanceUsage {
var usageRecords []db.WorkspaceInstanceUsage

for attributionId, instances := range report {
Expand All @@ -247,7 +248,7 @@ func usageReportToUsageRecords(report UsageReport) []db.WorkspaceInstanceUsage {
AttributionID: attributionId,
StartedAt: instance.CreationTime.Time(),
StoppedAt: stoppedAt,
CreditsUsed: 0,
CreditsUsed: float64(pricer.CreditsUsedByInstance(&instance, now)),
Copy link
Member

Choose a reason for hiding this comment

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

about the float64: I faintly recall we report credits as ints to stripe, right? If yes, we probably should do the same here, and adjust the DB/gRPC model if they don't agree.

@andrew-farries WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The designs here internal show credits displayed with fractions.

But I think you are right in that we should be consistent in allowing the credits per minute for a workspace class be fractional but always bill and display whole numbers of credits.

I'll update the types in a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding was that we have to report total usage to Stripe as an int, but we should collect usage per session in fractional units to avoid rounding errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained here, @jldec is right in thinking that this should in fact remain a float.

GenerationId: 0,
Deleted: false,
})
Expand Down
7 changes: 4 additions & 3 deletions components/usage/pkg/controller/reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ func TestUsageReport_CreditSummaryForTeams(t *testing.T) {
}

func TestUsageReportConversionToDBUsageRecords(t *testing.T) {
maxStopTime := time.Date(2022, 05, 31, 23, 00, 00, 00, time.UTC)
teamID := uuid.New().String()
teamAttributionID := db.NewTeamAttributionID(teamID)
instanceId := uuid.New()
Expand Down Expand Up @@ -178,7 +179,7 @@ func TestUsageReportConversionToDBUsageRecords(t *testing.T) {
AttributionID: teamAttributionID,
StartedAt: creationTime.Time(),
StoppedAt: sql.NullTime{Time: stoppedTime.Time(), Valid: true},
CreditsUsed: 0,
CreditsUsed: 470,
GenerationId: 0,
}},
},
Expand All @@ -200,15 +201,15 @@ func TestUsageReportConversionToDBUsageRecords(t *testing.T) {
AttributionID: teamAttributionID,
StartedAt: creationTime.Time(),
StoppedAt: sql.NullTime{},
CreditsUsed: 0,
CreditsUsed: 470,
GenerationId: 0,
}},
},
}

for _, s := range scenarios {
t.Run(s.Name, func(t *testing.T) {
actual := usageReportToUsageRecords(s.Report)
actual := usageReportToUsageRecords(s.Report, DefaultWorkspacePricer, maxStopTime)
require.Equal(t, s.Expected, actual)
})
}
Expand Down
12 changes: 6 additions & 6 deletions components/usage/pkg/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func Start(cfg Config) error {
return fmt.Errorf("failed to establish database connection: %w", err)
}

pricer, err := controller.NewWorkspacePricer(cfg.CreditsPerMinuteByWorkspaceClass)
if err != nil {
return fmt.Errorf("failed to create workspace pricer: %w", err)
}

var billingController controller.BillingController = &controller.NoOpBillingController{}

if cfg.StripeCredentialsFile != "" {
Expand All @@ -56,11 +61,6 @@ func Start(cfg Config) error {
return fmt.Errorf("failed to initialize stripe client: %w", err)
}

pricer, err := controller.NewWorkspacePricer(cfg.CreditsPerMinuteByWorkspaceClass)
if err != nil {
return fmt.Errorf("failed to create workspace pricer: %w", err)
}

billingController = controller.NewStripeBillingController(c, pricer)
}

Expand All @@ -69,7 +69,7 @@ func Start(cfg Config) error {
return fmt.Errorf("failed to parse schedule duration: %w", err)
}

ctrl, err := controller.New(schedule, controller.NewUsageReconciler(conn, billingController))
ctrl, err := controller.New(schedule, controller.NewUsageReconciler(conn, pricer, billingController))
if err != nil {
return fmt.Errorf("failed to initialize usage controller: %w", err)
}
Expand Down