Skip to content

[usage] Filter out not started workspace instances #12696

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 1 commit into from
Sep 6, 2022
Merged

[usage] Filter out not started workspace instances #12696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 2 additions & 10 deletions components/usage/pkg/apiv1/usage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,8 @@ func TestInstanceToUsageRecords(t *testing.T) {
workspaceID := dbtest.GenerateWorkspaceID()
teamAttributionID := db.NewTeamAttributionID(teamID)
instanceId := uuid.New()
creationTime := db.NewVarcharTime(time.Date(2022, 05, 30, 00, 00, 00, 00, time.UTC))
startedTime := db.NewVarcharTime(time.Date(2022, 05, 30, 00, 01, 00, 00, time.UTC))
stoppingTime := db.NewVarcharTime(time.Date(2022, 06, 1, 1, 0, 0, 0, time.UTC))
stoppedTime := db.NewVarcharTime(time.Date(2022, 06, 1, 1, 1, 0, 0, time.UTC))

scenarios := []struct {
Name string
Expand All @@ -320,10 +318,8 @@ func TestInstanceToUsageRecords(t *testing.T) {
WorkspaceClass: defaultWorkspaceClass,
Type: db.WorkspaceType_Prebuild,
UsageAttributionID: teamAttributionID,
CreationTime: creationTime,
StartedTime: startedTime,
StoppingTime: stoppingTime,
StoppedTime: stoppedTime,
},
},
Expected: []db.WorkspaceInstanceUsage{{
Expand Down Expand Up @@ -352,10 +348,8 @@ func TestInstanceToUsageRecords(t *testing.T) {
Type: db.WorkspaceType_Regular,
WorkspaceID: workspaceID,
UsageAttributionID: teamAttributionID,
CreationTime: creationTime,
StartedTime: startedTime,
StoppingTime: db.VarcharTime{},
StoppedTime: db.VarcharTime{},
},
},
Expected: []db.WorkspaceInstanceUsage{{
Expand Down Expand Up @@ -402,7 +396,7 @@ func TestReportGenerator_GenerateUsageReport(t *testing.T) {
UsageAttributionID: db.NewTeamAttributionID(teamID.String()),
StartedTime: db.NewVarcharTime(time.Date(2022, 05, 30, 00, 01, 00, 00, time.UTC)),
}),
// No creation time, invalid record
// No creation time, invalid record, ignored
dbtest.NewWorkspaceInstance(t, db.WorkspaceInstance{
ID: uuid.New(),
UsageAttributionID: db.NewTeamAttributionID(teamID.String()),
Expand All @@ -426,7 +420,7 @@ func TestReportGenerator_GenerateUsageReport(t *testing.T) {
require.Equal(t, nowFunc(), report.GenerationTime)
require.Equal(t, startOfMay, report.From)
// require.Equal(t, startOfJune, report.To) TODO(gpl) This is not true anymore - does it really make sense to test for it?
require.Len(t, report.InvalidSessions, 1)
require.Len(t, report.InvalidSessions, 0)
require.Len(t, report.UsageRecords, 2)
}

Expand Down Expand Up @@ -654,7 +648,6 @@ func TestReconcileWithLedger(t *testing.T) {
WorkspaceClass: db.WorkspaceClass_Default,
Type: db.WorkspaceType_Regular,
UsageAttributionID: db.NewTeamAttributionID(uuid.New().String()),
CreationTime: db.NewVarcharTime(now.Add(1 * time.Minute)),
}

inserts, updates := reconcileUsageWithLedger([]db.WorkspaceInstanceForUsage{instance, instance}, nil, pricer, now)
Expand Down Expand Up @@ -685,7 +678,6 @@ func TestReconcileWithLedger(t *testing.T) {
WorkspaceClass: db.WorkspaceClass_Default,
Type: db.WorkspaceType_Regular,
UsageAttributionID: db.NewTeamAttributionID(uuid.New().String()),
CreationTime: db.NewVarcharTime(now.Add(1 * time.Minute)),
}

// the fields in the usage record deliberately do not match the instance, except for the Instance ID.
Expand Down
5 changes: 1 addition & 4 deletions components/usage/pkg/db/workspace_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func ListWorkspaceInstancesInRange(ctx context.Context, conn *gorm.DB, from, to
Where(
conn.Where("wsi.stoppingTime >= ?", TimeToISO8601(from)).Or("wsi.stoppingTime = ?", ""),
).
Where("wsi.startedTime != ?", "").
Copy link
Member

Choose a reason for hiding this comment

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

🎯

Copy link
Member Author

Choose a reason for hiding this comment

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

This is where we filter out any without a started time

Where("wsi.startedTime < ?", TimeToISO8601(to)).
Where("wsi.usageAttributionId != ?", "").
FindInBatches(&instancesInBatch, 1000, func(_ *gorm.DB, _ int) error {
Expand All @@ -155,10 +156,8 @@ func queryWorkspaceInstanceForUsage(ctx context.Context, conn *gorm.DB) *gorm.DB
"ws.type as workspaceType, " +
"wsi.workspaceClass as workspaceClass, " +
"wsi.usageAttributionId as usageAttributionId, " +
"wsi.creationTime as creationTime, " +
"wsi.startedTime as startedTime, " +
"wsi.stoppingTime as stoppingTime, " +
"wsi.stoppedTime as stoppedTime, " +
"ws.ownerId as ownerId, " +
"ws.id as workspaceId",
).
Expand Down Expand Up @@ -224,10 +223,8 @@ type WorkspaceInstanceForUsage struct {
Type WorkspaceType `gorm:"column:workspaceType;type:char;size:16;default:regular;" json:"workspaceType"`
UsageAttributionID AttributionID `gorm:"column:usageAttributionId;type:varchar;size:60;" json:"usageAttributionId"`

CreationTime VarcharTime `gorm:"column:creationTime;type:varchar;size:255;" json:"creationTime"`
Copy link
Member Author

Choose a reason for hiding this comment

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

Removes these fields as they are just confusing. They did not get removed originally when we switched to stoppingTime

StartedTime VarcharTime `gorm:"column:startedTime;type:varchar;size:255;" json:"startedTime"`
StoppingTime VarcharTime `gorm:"column:stoppingTime;type:varchar;size:255;" json:"stoppingTime"`
StoppedTime VarcharTime `gorm:"column:stoppedTime;type:varchar;size:255;" json:"stoppedTime"`
}

// WorkspaceRuntimeSeconds computes how long this WorkspaceInstance has been running.
Expand Down
4 changes: 0 additions & 4 deletions components/usage/pkg/db/workspace_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,8 @@ func TestListWorkspaceInstancesInRange_Fields(t *testing.T) {
WorkspaceClass: instance.WorkspaceClass,
Type: workspace.Type,
UsageAttributionID: instance.UsageAttributionID,
CreationTime: instance.CreationTime,
StartedTime: instance.StartedTime,
StoppingTime: instance.StoppingTime,
StoppedTime: instance.StoppedTime,
}, retrieved[0])
})

Expand Down Expand Up @@ -159,10 +157,8 @@ func TestListWorkspaceInstancesInRange_Fields(t *testing.T) {
WorkspaceClass: instance.WorkspaceClass,
Type: workspace.Type,
UsageAttributionID: instance.UsageAttributionID,
CreationTime: instance.CreationTime,
StartedTime: instance.StartedTime,
StoppingTime: instance.StoppingTime,
StoppedTime: instance.StoppedTime,
}, retrieved[0])
})

Expand Down