Allow teams to sign up for Usage-Based Billing via Stripe

[jankeromnes]

Description

Allow teams to sign up for Usage-Based Billing via Stripe.

Related Issue(s)

Fixes #10326

How to test

1. Join the "Gitpod" team: invite link (this enables the Usage-Based feature flag for you)
2. Create a new, different team ✨
3. Navigate to your Team Settings > Team Billing
4. Wait for the Usage-Based UI to appear above the Team Plans
5. Upgrade your team to usage-based by providing a fake credit card (e.g. 4242 4242 4242 4242, 4/24, 424)
6. This should successfully create a Stripe Customer for your team, visible in our Stripe Dashboard (credentials in 1Password)

No Team Billing	Add Credit Card	Active Team Billing	Stripe Dashboard

Release Notes

NONE

Documentation

• /werft with-payment=true

[jankeromnes]

TODO:

• Implement a StripeService to manage Stripe customers
• Implement server API for teams to sign up for usage-based billing via Stripe
• Implement a basic Usage-Based Billing UI for Teams
• Show a pending state when waiting for Stripe to create the customer
• Show a Stripe portal when clicking on Manage → (follow-up?)

Known issue:

• If you refresh the dashboard, the Usage-Based feature flag takes a while to be re-enabled (so, if you reload, the Usage-Based UI will be hidden for a few seconds, but otherwise still work well)

[easyCZ]

When interactions with 3rd party services fail, we shouldn't be propagating the errors directly to the client. We don't entirely control what is in the errors and may contain sensitive data. How is that handled here? I'd expect that we check for success/error and return appropriate obfuscated error messages upstream to the client.

[jankeromnes]

Agree -- we could avoid propagating to the user/client with a try/catch:

// Pseudo-code
try {
    await stripeThing();
} catch (error) {
    log.error("the full error for our logs", { error });
    throw new Error("Stripe thing failed for some reason ¯\_(ツ)_/¯");
}

[easyCZ]

Behaviour wise: works well and tested.

The popup to add credit-card logs a number of errors when making a POST https://play.google.com/log?format=json&hasfast=true&authuser=0 net::ERR_BLOCKED_BY_CLIENT. Can this be removed? Why does it happen?

If you wanted to, this PR could've been split into the following:

1. UI to show upgrade button which does nothing
2. Server-side APIs (behind a feature flag) which return Unimplemented when part of team gitpod, NotAllowed otherwise.
3. Server-side APIs actually implement behaviour
4. UIs which hook up the behaviour.

That way, you'd be able to keep the PRs smaller, and individual steps smaller. For example, in this case you'd get the review feedback on needing the feature flag guard server-side on the API setup, not on the full PR.

[jankeromnes]

@gtsiolis Had a quick look into how we can style Stripe's PaymentElement.

Here is how they suggest we do it:

1. Start by picking a theme

The 4 themes are:

stripe	night	flat	none

I think a good Minimum-Viable-Change could be:

• Use stripe or flat in Light theme
• Use night in Dark theme

2. Customize the theme using variables

Set variables like fontFamily and colorPrimary to broadly customize components appearing throughout each Element.

We could do this to further align the look & feel of Stripe Elements with our dashboard in broad strokes. 💡

3. If needed, fine-tune individual components and states using rules

With this, we can even drill down into specific styling issues that might remain after 1. and 2. 🔍

[jldec]

Works for me as well - Is there a way to navigate manually to the Stripe portal to manage billing/payment details (as a Gitpod user - not as the GitPod Stripe admin) ?

[jldec]

@easyCZ do you know why the feature flag feels slow ?

[jankeromnes]

Works for me as well

Thanks! I'm glad this works perfectly fine for everyone. 😊

Is there a way to navigate manually to the Stripe portal to manage billing/payment details (as a Gitpod user - not as the GitPod Stripe admin) ?

I don't think so, but I'm planning to implement the Stripe portal in the next PR once this is merged.

[easyCZ]

@easyCZ do you know why the feature flag feels slow ?

To me it looks like react is not being told the values have changed so it doesn't propagate as soon as it should. I'm no expert on react either so not entirely sure where this delay lies.
The request for feature flags is made early and completes fast so it should be available at that point for rendering. I'll try to debug a bit when I have time but others more versed in React can probs debug this faster :)

[easyCZ]

How does this work on the server? The implementation of getExperimentsClient() is

export function getExperimentsClient(): Client {
    // We have already instantiated a client, we can just re-use it.
    if (client !== undefined) {
        return client;
    }

    const host = window.location.hostname;
    if (host === "gitpod.io") {
        client = newProductionConfigCatClient();
    } else if (host === "gitpod-staging.com" || host.endsWith("gitpod-dev.com") || host.endsWith("gitpod-io-dev.com")) {
        client = newNonProductionConfigCatClient();
    } else {
        // We're gonna use a client which always returns the default value.
        client = newAlwaysReturningDefaultValueClient();
    }

    return client;
}

This uses the window.location.hostname which doesn't exist in the server context. How is this initialised?

[jankeromnes]

If feature flags don't work as expected in the server, I'd vote for not using them right now (a client-side feature flag is enough to hide this WIP feature), and open a separate issue about fixing the server-side feature flags so that we can use them.

[easyCZ]

@jankeromnes How about adding a pre-pr to make it work on server properly and then using it here? I'm happy to pair if that makes it easier. Boy/girl scout rules apply.

[jankeromnes]

@easyCZ That's the dashboard implementation, right?

The server-side implementation seems to be here: https://github.com/gitpod-io/gitpod/blob/main/components/server/src/experiments.ts

[easyCZ]

Interesting, wasn't aware this landed (and my IDE resolved the method to the dashboard one). This must be stray from another PR. It also lives under the root of server/src which it should not.

This will in fact work for server. However, there's a second problem with it as it hard-codes a single Client ID which is for the preview/staging environment, but not production.

Let's land as is, and fix it up in a follow-up PR. We'll need to move the file, but also inject the Client ID based on environment we're running in. The current version would also break self-hosted environments as it reaches out to an external source.

[jankeromnes]

Follow-up issue: #10524

[geropl]

If you refresh the dashboard, the Usage-Based feature flag takes a while to be re-enabled (so, if you reload, the Usage-Based UI will be hidden for a few seconds, but otherwise still work well)

This is not the case for me, works rather snappy now. 👍

[geropl]

LGTM, tested and works.

I leave it up to you @jankeromnes to address the "error leak" in a follow-up.

[jankeromnes]

Awesome, many thanks @geropl. ❤️

Adding a brief hold for the try/catches:

/hold

[easyCZ]

@jankeromnes Let me know when API layer error handling is in and I'll re-review.

[easyCZ]

Also, IMO you should return a 404 error when customer.id does not exist. Returning undefined prevents you classifying your errors and actually having reasonable metrics on what worked and didn't. This goes for setupIntent also.

[jankeromnes]

Hmm, interesting point. I agree on the 404 semantics. However, returning undefined currently means that the customer doesn't exist yet, and is handled that way in the client (different from "the Stripe API failed"). If we switch this around to a 404, we'd need to update the client as well to handle 404 as a valid case (i.e. "the team is not yet a Stripe customer").

[easyCZ]

I think that's correct. If it doesn't exist, then it wasn't found and we should create one (if needed). In general, we should try to avoid OK responses with No data as much as possible, especially when we know why there is no data - in this case it doesn't exist in stripe.

[easyCZ]

(Follow-up PR/issue is fine)

[jankeromnes]

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-jx-stripe-service.15
(with .werft/ from main)

[jankeromnes]

Thanks again for the helpful reviews 🙏 and sorry for the bumpy ride. Will try even harder to keep PRs small going forward.

Build finally went through, and everything still works. 🎉

Pushing this over the finish line. 🏁 Many improvements to come as follow-ups. 🚀

/unhold