Skip to content

[experiments] Add abstraction for configcat to work in self-hosted #10807

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

Jump to bottom
Merged
merged 1 commit into from
Jun 27, 2022
Merged

[experiments] Add abstraction for configcat to work in self-hosted #10807

merged 1 commit into from
Jun 27, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Jun 21, 2022 •
edited by geropl
Loading

Description

Adds an interface in front of configcat. This allows the following:

  1. Different SDK keys to be used for different environments (specified as ENV variable)
  2. When not specified, uses a default implementation which always returns the default value. This is essential to not break air-gapped environments for self-hosted.

We use a shared wrapper interface for experiments implemented in @gitpod-protocol. We then either use a browser version of configcat client for dashboard, or configcat-node for backend.

Related Issue(s)

Fixed: #10524

How to test

  1. Start a preview environment
  2. Create team Gitpod
  3. Try to configure usage based pricing - if you can see it, and configure it then this PR works

Release Notes

NONE

Documentation

NONE

  • /werft with-preview

@easyCZ easyCZ requested a review from a team June 21, 2022 20:11
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 21, 2022
@easyCZ
Copy link
Member Author

easyCZ commented Jun 21, 2022

/hold
There's an open question I need help with before I can land this.

@easyCZ easyCZ force-pushed the mp/server-experiments branch from 8347b2d to bb62147 Compare June 22, 2022 09:36
@easyCZ
Copy link
Member Author

easyCZ commented Jun 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-mp-server-experiments.7
(with .werft/ from main)

@easyCZ
Copy link
Member Author

easyCZ commented Jun 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-mp-server-experiments.9
(with .werft/ from main)

@easyCZ
Copy link
Member Author

easyCZ commented Jun 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-mp-server-experiments.10
(with .werft/ from main)

@easyCZ easyCZ force-pushed the mp/server-experiments branch 2 times, most recently from 7e2c47e to b4a2d9f Compare June 22, 2022 12:05
@easyCZ easyCZ mentioned this pull request Jun 22, 2022
Merged
@easyCZ
Copy link
Member Author

easyCZ commented Jun 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-mp-server-experiments.13
(with .werft/ from main)

@easyCZ easyCZ force-pushed the mp/server-experiments branch from b4a2d9f to ad6ac5f Compare June 22, 2022 12:59
This was referenced Jun 23, 2022
Closed
Merged
@easyCZ easyCZ force-pushed the mp/server-experiments branch from 11623f4 to a951aa4 Compare June 24, 2022 11:44
@roboquat roboquat added size/XL and removed size/L labels Jun 24, 2022
easyCZ
easyCZ commented Jun 24, 2022
View reviewed changes
components/gitpod-protocol/src/experiments/configcat-server.ts

const configCatClient = configcat.createClient(sdkKey, {
logger: configcat.createConsoleLogger(LogLevel.Error),
maxInitWaitTimeSeconds: 0,
Copy link
Member Author

Choose a reason for hiding this comment

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

@jankeromnes I've pulled in your change into my refactor as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Is this ready for review?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is :)

@easyCZ
Copy link
Member Author

easyCZ commented Jun 24, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview

👍 started the job as gitpod-build-mp-server-experiments.17
(with .werft/ from main)

@easyCZ easyCZ force-pushed the mp/server-experiments branch from a951aa4 to 82b32e5 Compare June 24, 2022 12:12
@easyCZ
Copy link
Member Author

easyCZ commented Jun 24, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview

👍 started the job as gitpod-build-mp-server-experiments.19
(with .werft/ from main)

@easyCZ
Copy link
Member Author

easyCZ commented Jun 24, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview with-payment

👍 started the job as gitpod-build-mp-server-experiments.20
(with .werft/ from main)

@easyCZ
Copy link
Member Author

easyCZ commented Jun 24, 2022

Can confirm this works in preview
Screenshot 2022-06-24 at 14 37 26

geropl
geropl reviewed Jun 24, 2022
View reviewed changes
geropl
geropl approved these changes Jun 24, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested and works.

/hold in case you want to have @jankeromnes take another round.

@geropl geropl self-assigned this Jun 24, 2022
@easyCZ easyCZ force-pushed the mp/server-experiments branch from 82b32e5 to 05b2246 Compare June 27, 2022 07:28
@easyCZ
Copy link
Member Author

easyCZ commented Jun 27, 2022

/unhold

@roboquat roboquat merged commit 12b92e9 into main Jun 27, 2022
@roboquat roboquat deleted the mp/server-experiments branch June 27, 2022 07:40
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jankeromnes jankeromnes jankeromnes left review comments

@geropl geropl geropl approved these changes

Assignees

@geropl geropl

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve server-side feature flags
4 participants
@easyCZ @jankeromnes @geropl @roboquat