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

feat: create Feature class for handling feature rollouts #41

Merged
merged 5 commits into from
Sep 25, 2023

Conversation

matt-codecov
Copy link
Contributor

@matt-codecov matt-codecov commented Sep 20, 2023

this came up on slack yesterday and i thought it'd be fun to throw something together. let me know if you have any feedback or if we want to use a Real Product for this instead

notes:

  • it can nominally support A/B tests, but our data isn't prepared to support analysis. mostly meant for simple rollouts at the moment
  • increasing a rollout requires a code change and new deploy which is not ideal. an alternative could be to read the rollout proportions from redis and cache for, say, 4h, if we have a good way to update values in production redis
  • i used the 128-bit hash because it was there, but at our volumes we are probably safe from the birthday problem with 64 bits? i didn't crunch the numbers though, just going on vibes
  • the bucketing logic uses floats with int truncation which a serious statistician may find introduces biases but it should hold up for now
# Simple feature rolled out to 20% of repos
# Override codecov repos into the feature early
# By default, features have a single variant:
#   {"enabled": FeatureVariant(True, 1.0)}
MY_FEATURE_BY_REPO = Feature(
    "my_feature",
    0.2,
    overrides={
        "{shared repo id}": "enabled",
        "{worker repo id}": "enabled",
        "{codecov-api repo id}": "enabled",
        "{gazebo repo id}": "enabled",
        "{codecov-cli repo id}": "enabled",
)
if MY_FEATURE_BY_REPO.check_value(repo_id, default=False):
    new_behavior()

# Simple A/B test rolled out to 10% of users (5% control, 5% test)
MY_EXPERIMENT_BY_USER = Feature(
    "my_experiment",
    0.1,
    variants={
        "test": FeatureVariant(True, 0.5),
        "control": FeatureVariant(False, 0.5),
    },
)
if MY_EXPERIMENT_BY_USER.check_value(user_id, default=False):
    new_behavior()

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #41 (8b997c8) into main (15a7041) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   91.84%   91.88%   +0.04%     
==========================================
  Files         100      101       +1     
  Lines        8249     8290      +41     
  Branches     1253     1261       +8     
==========================================
+ Hits         7576     7617      +41     
  Misses        671      671              
  Partials        2        2              
Flag Coverage Δ
python3.10.5 88.67% <100.00%> (+0.07%) ⬆️
python3.8.13 88.76% <100.00%> (+0.07%) ⬆️
python3.9.12 89.09% <100.00%> (+0.07%) ⬆️
rust 90.17% <ø> (ø)
smart-labels 92.25% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
shared/rollouts/__init__.py 100.00% <100.00%> (ø)

@giovanni-guidini
Copy link
Contributor

This is an interesting project, thanks for putting it together.

I'm particularly interested in the overrides right now. Will it be possible to pass an org_id and have the feature be enabled to all of the org? (for the case we want to test things in codecov first)

Also, I don't have to define my Features and Experiments in shared, do I? That can be done in api or worker directly?

@matt-codecov
Copy link
Contributor Author

note to self: i require proportions be >0 but it should be >=0 to allow for override-only features and feature variants

Will it be possible to pass an org_id and have the feature be enabled to all of the org?

yeah, if you set an override for codecov's org id and then call check_value() with an org id, it'll return the override value. it isn't smart enough to go from a repo id to its owner, so it's recommended you name features like MY_FEATURE_BY_ORG to remind yourself you should pass an org id in

Also, I don't have to define my Features and Experiments in shared, do I? That can be done in api or worker directly?

yeah, i think a central features.py would be nice so we can easily know how much runtime variability there is / we can look for suspicious feature rollouts when customers report problems but we can do that per-repo

Copy link
Contributor

@giovanni-guidini giovanni-guidini left a comment

Choose a reason for hiding this comment

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

I like in-house solutions 🤷‍♂️ hehehe
Thanks

(this is only my opinion, of course. We should wait to hear from others)

@matt-codecov
Copy link
Contributor Author

functools.cached_property was new in python 3.8 so it's failing to build in 3.7. do we actually ship 3.7 anywhere?

Copy link
Contributor

@scott-codecov scott-codecov left a comment

Choose a reason for hiding this comment

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

This is nice @matt-codecov!

There's currently some %-based rollouts that are happening via the code here: https://github.com/codecov/shared/blob/071b899ae80c7a465b4e6d7d8387d9dff241e842/shared/yaml/user_yaml.py#L142C1-L142C1 (and configured here: https://github.com/codecov/k8s-v2/blob/3cc3da33deba0eb0dbd6586a11d5f034b9839acb/production.yaml#L391). They have slightly different percentages in different envs. If we need to continue supporting that sort of thing then maybe we come up with a convention for specifying variant proportions in the config.

@@ -0,0 +1,3 @@
PARALLEL_UPLOAD_PROCESSING_BY_ORG = Feature(
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wanted a "real" example feature to serve as a reference and placeholder, but for that actual experiment i'll probably put the feature definition in worker. i'll comment it out / explain in that file

@scott-codecov
Copy link
Contributor

functools.cached_property was new in python 3.8 so it's failing to build in 3.7. do we actually ship 3.7 anywhere?

I don't believe so. Python 3.7 is end-of-life anyway so I think we're fine to drop that version here.

@matt-codecov
Copy link
Contributor Author

apparently my last merge was slightly too quick to get the CI config update i merged

@matt-codecov matt-codecov merged commit 7807e19 into main Sep 25, 2023
12 checks passed
@matt-codecov matt-codecov deleted the matt/feature-rollout-helper branch September 25, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants