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

Fix AccountStatementProvider #12431

Merged
merged 2 commits into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export class GarbageCollectedCache<T> {
if (!entry) {
return undefined;
}
// Still valid?
if (entry.expiryDate < Date.now()) {
this.store.delete(entry.key);
return undefined;
}
geropl marked this conversation as resolved.
Show resolved Hide resolved
return entry.value;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,7 @@ export class EntitlementServiceChargebee implements EntitlementService {
hasHitParallelWorkspaceLimit(),
]);

const result = enoughCredits && !hitParallelWorkspaceLimit;

console.log("mayStartWorkspace > hitParallelWorkspaceLimit " + hitParallelWorkspaceLimit);

return {
mayStart: result,
oufOfCredits: !enoughCredits,
hitParallelWorkspaceLimit,
};
Expand All @@ -84,10 +79,9 @@ export class EntitlementServiceChargebee implements EntitlementService {
runningInstances: Promise<WorkspaceInstance[]>,
): Promise<boolean> {
// As retrieving a full AccountStatement is expensive we want to cache it as much as possible.
const cachedAccountStatement = this.accountStatementProvider.getCachedStatement();
const cachedAccountStatement = this.accountStatementProvider.getCachedStatement(userId);
const lowerBound = this.getRemainingUsageHoursLowerBound(cachedAccountStatement, date.toISOString());
if (lowerBound && (lowerBound === "unlimited" || lowerBound > Accounting.MINIMUM_CREDIT_FOR_OPEN_IN_HOURS)) {
console.log("checkEnoughCreditForWorkspaceStart > unlimited");
return true;
}

Expand All @@ -96,7 +90,6 @@ export class EntitlementServiceChargebee implements EntitlementService {
date.toISOString(),
runningInstances,
);
console.log("checkEnoughCreditForWorkspaceStart > remainingUsageHours " + remainingUsageHours);
return remainingUsageHours > Accounting.MINIMUM_CREDIT_FOR_OPEN_IN_HOURS;
}

Expand All @@ -115,7 +108,7 @@ export class EntitlementServiceChargebee implements EntitlementService {
return "unlimited";
}

const diffInMillis = new Date(cachedStatement.endDate).getTime() - new Date(date).getTime();
const diffInMillis = Math.max(0, new Date(cachedStatement.endDate).getTime() - new Date(date).getTime());
geropl marked this conversation as resolved.
Show resolved Hide resolved
const maxPossibleUsage = millisecondsToHours(diffInMillis) * MAX_PARALLEL_WORKSPACES;
return cachedStatement.remainingHours - maxPossibleUsage;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class EntitlementServiceLicense implements EntitlementService {
runningInstances: Promise<WorkspaceInstance[]>,
): Promise<MayStartWorkspaceResult> {
// if payment is not enabled users can start as many parallel workspaces as they want
return { mayStart: true };
return {};
}

async maySetTimeout(user: User, date: Date): Promise<boolean> {
Expand Down
2 changes: 0 additions & 2 deletions components/server/ee/src/billing/entitlement-service-ubp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ export class EntitlementServiceUBP implements EntitlementService {
this.checkSpendingLimitReached(user, date),
hasHitParallelWorkspaceLimit(),
]);
const result = !spendingLimitReachedOnCostCenter && !hitParallelWorkspaceLimit;
return {
mayStart: result,
spendingLimitReachedOnCostCenter,
hitParallelWorkspaceLimit,
};
Expand Down
14 changes: 4 additions & 10 deletions components/server/ee/src/billing/entitlement-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,23 @@ export class EntitlementServiceImpl implements EntitlementService {
const verification = await this.verificationService.needsVerification(user);
if (verification) {
return {
mayStart: false,
needsVerification: true,
};
}
const billingMode = await this.billingModes.getBillingModeForUser(user, date);
let result;
switch (billingMode.mode) {
case "none":
result = await this.license.mayStartWorkspace(user, date, runningInstances);
break;
return this.license.mayStartWorkspace(user, date, runningInstances);
case "chargebee":
result = await this.chargebee.mayStartWorkspace(user, date, runningInstances);
break;
return this.chargebee.mayStartWorkspace(user, date, runningInstances);
case "usage-based":
result = await this.ubp.mayStartWorkspace(user, date, runningInstances);
break;
return this.ubp.mayStartWorkspace(user, date, runningInstances);
default:
throw new Error("Unsupported billing mode: " + (billingMode as any).mode); // safety net
}
return result;
} catch (err) {
log.error({ userId: user.id }, "EntitlementService error: mayStartWorkspace", err);
throw err;
return {}; // When there is an EntitlementService error, we never want to break workspace starts
}
}

Expand Down
2 changes: 1 addition & 1 deletion components/server/ee/src/container-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ export const productionEEContainerModule = new ContainerModule((bind, unbind, is
// GitpodServerImpl (stateful per user)
rebind(GitpodServerImpl).to(GitpodServerEEImpl).inRequestScope();
bind(EligibilityService).toSelf().inRequestScope();
bind(AccountStatementProvider).toSelf().inRequestScope();

// various
rebind(HostContainerMapping).to(HostContainerMappingEE).inSingletonScope();
bind(EMailDomainService).to(EMailDomainServiceImpl).inSingletonScope();
rebind(BlockedUserFilter).toService(EMailDomainService);
bind(SnapshotService).toSelf().inSingletonScope();
bind(AccountStatementProvider).toSelf().inSingletonScope();

bind(UserDeletionServiceEE).toSelf().inSingletonScope();
rebind(UserDeletionService).to(UserDeletionServiceEE).inSingletonScope();
Expand Down
17 changes: 8 additions & 9 deletions components/server/ee/src/user/account-statement-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { injectable, inject } from "inversify";
import { WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { AccountStatement } from "@gitpod/gitpod-protocol/lib/accounting-protocol";
import { AccountService } from "@gitpod/gitpod-payment-endpoint/lib/accounting";
import { GarbageCollectedCache } from "@gitpod/gitpod-protocol/lib/util/garbage-collected-cache";

export type CachedAccountStatement = Pick<AccountStatement, "remainingHours" | "endDate">;

Expand All @@ -19,20 +20,18 @@ export type CachedAccountStatement = Pick<AccountStatement, "remainingHours" | "
export class AccountStatementProvider {
@inject(AccountService) protected readonly accountService: AccountService;

protected cachedStatement: CachedAccountStatement | undefined;
/**
* AccountStatements, cached by userId
*/
protected readonly cachedStatements = new GarbageCollectedCache<CachedAccountStatement>(5 * 60, 10 * 60);

setCachedStatement(cachedStatement: CachedAccountStatement) {
this.cachedStatement = cachedStatement;
}

getCachedStatement(): CachedAccountStatement | undefined {
return this.cachedStatement;
getCachedStatement(userId: string): CachedAccountStatement | undefined {
return this.cachedStatements.get(userId);
}

async getAccountStatement(userId: string, date: string): Promise<AccountStatement> {
const statement = await this.accountService.getAccountStatement(userId, date);
// Fill cache
this.setCachedStatement({
this.cachedStatements.set(userId, {
remainingHours: statement.remainingHours,
endDate: statement.endDate,
});
Expand Down
15 changes: 7 additions & 8 deletions components/server/ee/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ import { UserCounter } from "../user/user-counter";
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
import { CachingUsageServiceClientProvider, UsageService } from "@gitpod/usage-api/lib/usage/v1/sugar";
import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb";
import { EntitlementService } from "../../../src/billing/entitlement-service";
import { EntitlementService, MayStartWorkspaceResult } from "../../../src/billing/entitlement-service";
import { BillingMode } from "@gitpod/gitpod-protocol/lib/billing-mode";
import { BillingModes } from "../billing/billing-mode";
import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
Expand Down Expand Up @@ -256,15 +256,14 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
): Promise<void> {
await super.mayStartWorkspace(ctx, user, runningInstances);

let result;
let result: MayStartWorkspaceResult = {};
try {
result = await this.entitlementService.mayStartWorkspace(user, new Date(), runningInstances);
} catch (error) {
throw new ResponseError(ErrorCodes.INTERNAL_SERVER_ERROR, `Error in Entitlement Service.`);
}
log.info("mayStartWorkspace", { result });
if (result.mayStart) {
return; // green light from entitlement service
TraceContext.addNestedTags(ctx, { mayStartWorkspace: { result } });
} catch (err) {
log.error({ userId: user.id }, "EntitlementSerivce.mayStartWorkspace error", err);
TraceContext.setError(ctx, err);
return; // we don't want to block workspace starts because of internal errors
geropl marked this conversation as resolved.
Show resolved Hide resolved
}
if (!!result.needsVerification) {
throw new ResponseError(ErrorCodes.NEEDS_VERIFICATION, `Please verify your account.`);
Expand Down
5 changes: 1 addition & 4 deletions components/server/src/billing/entitlement-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
import { injectable } from "inversify";

export interface MayStartWorkspaceResult {
mayStart: boolean;

hitParallelWorkspaceLimit?: HitParallelWorkspaceLimit;

oufOfCredits?: boolean;

needsVerification?: boolean;
Expand Down Expand Up @@ -83,7 +80,7 @@ export class CommunityEntitlementService implements EntitlementService {
date: Date,
runningInstances: Promise<WorkspaceInstance[]>,
): Promise<MayStartWorkspaceResult> {
return { mayStart: true };
geropl marked this conversation as resolved.
Show resolved Hide resolved
return {};
}

async maySetTimeout(user: User, date: Date): Promise<boolean> {
Expand Down