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

[usage] Implement BillingService #11655

Merged
merged 1 commit into from
Jul 27, 2022
Merged

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Jul 26, 2022

Description

Implements UpdateInvoices RPC on BillingService.

There are 2 implementations:

  1. NoOp implementation for Self-hosted
  2. Stripe implementation which re-uses the existing stripe.Client

With this PR, the flow changes to the following:

  1. Usage Controller is triggered with a cron
  2. Usage is retrieved and stored in the DB
  3. Instead of then triggering the old BillingController, the BillingService.UpdateInvoices(sessions) is invoked over a grpc self-connection

Behaviour of the component remains unchanged. Tests are updated to move to the corresponding parts.

The benefit of using na RPC is that we get metrics for the RPCs but also we incrementally decouple the UsageController from the Billing one. In subsequent PRs, a similar RPC will be introduced for collecting Usage and these two RPCs will then be chained. We started with Billing as it's easier to do initially and reduces the size of subsequent PRs. If you want to see how this looks, here's a PoC which converts both.

Related Issue(s)

Relates to #10785

How to test

Unit tests
Preview

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview with-billing

@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch from 9f59205 to 65aaa25 Compare July 26, 2022 13:40
@easyCZ easyCZ changed the title [usage] Add BillingService with UpdateInvoices RPC [usage] Implement BillingService Jul 26, 2022
@easyCZ easyCZ changed the base branch from main to mp/usage-billing-service July 26, 2022 13:42
@roboquat roboquat added size/L and removed size/XXL labels Jul 26, 2022
Base automatically changed from mp/usage-billing-service to main July 26, 2022 14:23
@roboquat roboquat added size/XXL and removed size/L labels Jul 26, 2022
@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch from 283117f to c8bed3e Compare July 26, 2022 21:19
@roboquat roboquat added size/XL and removed size/XXL labels Jul 26, 2022
@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch from c8bed3e to f905ecf Compare July 26, 2022 21:33
@easyCZ
Copy link
Member Author

easyCZ commented Jul 26, 2022

/werft run with-preview with-billing

👍 started the job as gitpod-build-mp-usage-billing-service-impl.7
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-mp-usage-billing-service-impl.8 because the annotations in the pull request description changed
(with .werft/ from main)

@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch from f905ecf to a84798d Compare July 26, 2022 21:35
@easyCZ easyCZ marked this pull request as ready for review July 26, 2022 21:40
@easyCZ easyCZ requested a review from a team July 26, 2022 21:40
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 26, 2022
@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch 3 times, most recently from a508cee to 492699c Compare July 27, 2022 07:24
@easyCZ
Copy link
Member Author

easyCZ commented Jul 27, 2022

/werft run with-preview with-billing

👍 started the job as gitpod-build-mp-usage-billing-service-impl.13
(with .werft/ from main)

Copy link
Contributor

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

/hold for one extra test

@easyCZ easyCZ force-pushed the mp/usage-billing-service-impl branch from 492699c to 1fd7237 Compare July 27, 2022 07:40
@easyCZ
Copy link
Member Author

easyCZ commented Jul 27, 2022

/hold
For me to validate one part of this change.

@easyCZ
Copy link
Member Author

easyCZ commented Jul 27, 2022

/unhold

@roboquat roboquat merged commit c4708b3 into main Jul 27, 2022
@roboquat roboquat deleted the mp/usage-billing-service-impl branch July 27, 2022 07:48
@andrew-farries
Copy link
Contributor

FYI, this looks like it's failing to establish the gRPC connection to itself in staging:

{"config":{"controllerSchedule":"1h","creditsPerMinuteByWorkspaceClass":{"default":0.1666666667,"gitpodio-internal-xl":0.3333333333},"stripeCredentialsFile":"stripe-secret/apikeys","contentServiceAddress":"content-service:8080","server":{"services":{"grpc":{"address":":9001"}}}},"level":"info","message":"Starting usage component.","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"INFO","time":"2022-07-27T09:32:20Z"}
{"level":"info","message":"Starting usage controller.","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"INFO","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"2022/07/27 09:32:20 WARNING: [core] [Channel #2 SubChannel #3] grpc: addrConn.createTransport failed to connect to {","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"Addr\": \":9001\",","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"ServerName\": \"localhost:9001\",","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"Attributes\": null,","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"BalancerAttributes\": null,","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"Type\": 0,","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"  \"Metadata\": null","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}
{"component":"grpc","level":"warning","message":"}. Err: connection error: desc = \"transport: Error while dialing dial tcp :9001: connect: connection refused\"","serviceContext":{"service":"usage","version":"commit-c4708b30483eae2bcd07144178252291ea97c326"},"severity":"WARNING","time":"2022-07-27T09:32:20Z"}

@andrew-farries
Copy link
Contributor

andrew-farries commented Jul 27, 2022

although, after restarting the pod it seems fine 🤷

@easyCZ
Copy link
Member Author

easyCZ commented Jul 27, 2022

Will take a closer look. It's possible it's a race condition between the server starting and the connection.

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 28, 2022
@andrew-farries
Copy link
Contributor

I made #11743 to track the race at startup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XL team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants