Skip to content

Commit

Permalink
Migrate to per-study file structure. (#1234)
Browse files Browse the repository at this point in the history
Update instructions, create per-study file structure, switch workflows
to use the new generator.

This PR currently requires both seeds to be in sync. Once it's merged
and the production CI is updated, the restriction will be lifted, and
the old seed will no longer be used.
  • Loading branch information
goodov authored Oct 28, 2024
1 parent 885cb07 commit 9dc1c13
Show file tree
Hide file tree
Showing 84 changed files with 3,154 additions and 33 deletions.
12 changes: 9 additions & 3 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
# Griffin maintainers are responsible for all machinery in this repository.
* @brave/griffin-maintainers

# The seed file is owned by the uplift team as any change to it directly affects
# public builds. It's important to update the seed in a controlled manner.
/seed/seed.json @brave/uplift-approvers
# The seed directory is deprecated and no longer used for the seed generation.
# However, it is used by the perf infrastructure to access the Griffin seed at
# some point in the past. Lock it to Griffin maintainers to prevent any
# unrelated changes.
/seed/** @brave/griffin-maintainers

# The studies are owned by the uplift team as any change directly affects public
# builds. It's important to update studies in a controlled manner.
/studies/*.json5 @brave/uplift-approvers
14 changes: 1 addition & 13 deletions .github/workflows/generate-test-seed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
pull_request:
paths:
- '.github/workflows/generate-test-seed.yml'
- 'seed/seed.json'
- 'studies/**'

jobs:
Expand All @@ -23,14 +22,6 @@ jobs:
- name: Fetch base commit
run: git fetch --depth=1 origin "$BASE_SHA"

- name: Setup Python
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5
with:
python-version: '3.11'

- name: Install python requirements
run: pip install -r seed/requirements.txt

- name: Comment "Generation In Progress"
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
with:
Expand Down Expand Up @@ -64,10 +55,7 @@ jobs:

- name: Generate seed
run: |
# Use only python implementation for now.
python seed/serialize.py seed/seed.json --version "$SEED_VERSION"
# TODO: enable this when per-file studies will be synced with seed.json.
# npm run seed_tools -- create_seed studies seed.bin --version "$SEED_VERSION"
npm run seed_tools create -- --version "$SEED_VERSION"
- name: Upload seed
env:
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/test-src.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,14 @@ jobs:

- name: build
run: npm run build

- name: Setup Python
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5
with:
python-version: '3.11'

- name: Install python seed generator requirements
run: pip install -r seed/requirements.txt

- name: Compare python and typescript seed generator
run: npm run seed_tools compare_python_gen
28 changes: 17 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,42 @@ See the [Wiki](<https://github.com/brave/brave-browser/wiki/Brave-Variations-(Gr
A continuous integration server (CI) serializes and signs the updated seed file before publishing it to a CDN endpoint at https://variations.brave.com/seed. To browse the contents of the seed file a dashboard is hosted at https://griffin.brave.com. The repo is organized as follows:

- `/crypto` contains a util to create key pairs and sign the seed file.
- `/seed` contains the JSON seed definition and serialisation code.
- `/src` contains the web dashboard to browse the seed contents and tracker code the track changes. See https://github.com/brave/brave-variations/blob/main/src/README.md for details.
- `/seed` contains a deprecated JSON seed definition and serialisation code.
- `/src` contains the web dashboard to browse the seed contents, the tracker code to track seed changes and the current seed generator. See [src/README.md](src/README.md) for details.
- `/studies` contains the studies used to generate the seed.

## Git flow
## Git flow for `/studies`

1. Work in feature branch and when done create a PR to `main` branch.
2. Verify that everything works as intended via PR instructions (use `--variations-pr` flag).
1. Run `npm install` after a checkout.
2. Create or modify a study in `/studies`, following the protobuf schema in
[`src/proto/study.proto`](/src/proto/study.proto).
3. Run `npm run seed_tools lint -- --fix` until there are no issues.
4. Create a PR to the `main` branch.
5. Verify that everything works as intended using the PR instructions.

## Key Generation and Exchange

On initial deployment and subsequent key rotations a new key pair has to be generated. The public key is exchanged by patching the hard-coded public key bytes in [variations_seed_store.cc#L37](https://source.chromium.org/chromium/chromium/src/+/master:components/variations/variations_seed_store.cc;l=37):
On initial deployment and subsequent key rotations a new key pair has to be generated. The public key is exchanged by patching the hard-coded public key bytes in [variations_seed_store.cc](https://source.chromium.org/chromium/chromium/src/+/main:components/variations/variations_seed_store.cc;l=54;drc=5e751800f8e981ee6a18db8a8fa00883a851ecf7):

1. Generate a key pair with `$ go run ./crypto/crypto_util.go keygen`.
2. Update the [patched public key](https://github.com/brave/brave-core/blob/master/chromium_src/components/variations/variations_seed_store.cc#L6) in brave-core.
2. Update the [patched public key](https://github.com/brave/brave-core/blob/e2ded454bd2153e8e1a46be7ae13bbd540cf0d6d/chromium_src/components/variations/variations_seed_store.cc#L44-L53) in brave-core.
3. Store the private key in a secure vault and ensure it is accessible by CI.

## Seed Serialization, Signing and Serving

The following steps are performed by CI to publish the updated seed file:

1. Run `$ python seed/serialize.py seed/seed.json` to compile the protobuf.
1. Run `$ npm run seed_tools create` to compile the protobuf.
2. Sign the seed file with `$ go run /crypto/crypto_util.go sign`.
3. Update the `X-Seed-Signature` response header.
4. Update the ETAG header with the contents of `serialnumber`.
5. Gzip the seed and set `Content-Encoding: gzip` response header.

Constraints:

- All studies are [one time randomized](https://source.chromium.org/chromium/chromium/src/+/main:base/metrics/field_trial.h;l=99).
- Platform and channel filters must be applied. See `PLATFORMS` and `CHANNELS` constants in `serialize.py`.
- Brave Ads studies must contain the stubstring "BraveAds" in their study name. Only one ads study with page visible side effects is allowed to run. Multiple studies without visible side effects can run simultanesouly.
- All studies are [one time randomized](https://source.chromium.org/chromium/chromium/src/+/main:base/metrics/field_trial.h;l=43;drc=60a72b0afdb415164c8f72cb0cada4317e4464a1).
- Platform and channel filters must be applied.
- Brave Ads studies must contain the substring "BraveAds" in their study name. Only one ads study with page visible side effects is allowed to run. Multiple studies without visible side effects can run simultanesouly.

## Some Notes on using variations in the Browser

Expand Down
10 changes: 6 additions & 4 deletions src/README.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
# Griffin & Finch Tracker
# Griffin & Finch Tracker & Seed Tools

## Directory structure

`core` is common Typescript code used by both Tracker and griffin.brave.com.

`finch_tracker` - NodeJS/TS console app to track seed changes. See https://github.com/brave/finch-data-private/#finch-tracker

`web` WebUI hosted on griffin.brave.com. It parses raw seed data and shows them in human readable format. Doesn't use any sophisticated backend, 100% code runs on the client side.
`proto` chromium protobuf files describing seed data format.

`test` is supporting code/data to use in tests
`seed_tools` seed generator and related tools to create the seed from `/studies` directory.

`proto` chromium protobuf files describing seed data format.
`test` is supporting code/data to use in tests.

`web` WebUI hosted on griffin.brave.com. It parses raw seed data and shows them in human readable format. Doesn't use any sophisticated backend, 100% code runs on the client side.

## Commands and actions

Expand Down
3 changes: 1 addition & 2 deletions src/scripts/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ function getLintAllCommands(options: Options): string[] {
return [
'prettier . --ignore-unknown' + (options.fix ? ' --write' : ' --check'),
'eslint . --config src/.eslintrc.js' + (options.fix ? ' --fix' : ''),
// TODO(goodov): Enable when per-file structure appears.
// 'npm run seed_tools lint --' + (options.fix ? ' --fix' : ''),
'npm run seed_tools lint --' + (options.fix ? ' --fix' : ''),
];
}

Expand Down
34 changes: 34 additions & 0 deletions studies/AllowCertainClientHintsStudy.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
[
{
name: 'AllowCertainClientHintsStudy',
experiment: [
{
name: 'Enabled',
probability_weight: 100,
feature_association: {
enable_feature: [
'AllowCertainClientHints',
],
},
},
{
name: 'Default',
probability_weight: 0,
},
],
filter: {
min_version: '104.1.44.59',
channel: [
'RELEASE',
'BETA',
'NIGHTLY',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
'ANDROID',
],
},
},
]
31 changes: 31 additions & 0 deletions studies/BraveAIChatEnabledStudy.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
[
{
name: 'BraveAIChatEnabledStudy',
experiment: [
{
name: 'Enabled',
probability_weight: 100,
feature_association: {
enable_feature: [
'AIChat',
],
},
},
{
name: 'Default',
probability_weight: 0,
},
],
filter: {
min_version: '119.1.60.0',
channel: [
'RELEASE',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
],
},
},
]
33 changes: 33 additions & 0 deletions studies/BraveAdblockExperimentalListDefaultStudy.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
[
{
name: 'BraveAdblockExperimentalListDefaultStudy',
experiment: [
{
name: 'Enabled',
probability_weight: 100,
feature_association: {
enable_feature: [
'BraveAdblockExperimentalListDefault',
],
},
},
{
name: 'Default',
probability_weight: 0,
},
],
filter: {
min_version: '123.1.66.53',
channel: [
'BETA',
'NIGHTLY',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
'ANDROID',
],
},
},
]
33 changes: 33 additions & 0 deletions studies/BraveAdblockMobileNotificationsListDefault.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
[
{
name: 'BraveAdblockMobileNotificationsListDefault',
experiment: [
{
name: 'Enabled',
probability_weight: 100,
feature_association: {
enable_feature: [
'BraveAdblockMobileNotificationsListDefault',
],
},
},
{
name: 'Default',
probability_weight: 0,
},
],
filter: {
channel: [
'RELEASE',
'BETA',
'NIGHTLY',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
'ANDROID',
],
},
},
]
43 changes: 43 additions & 0 deletions studies/BraveAds.CreativeAdModelBasedPredictorRecencyStudy.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
[
{
name: 'BraveAds.CreativeAdModelBasedPredictorRecencyStudy',
experiment: [
{
name: 'NoRecency',
probability_weight: 100,
feature_association: {
enable_feature: [
'CreativeNotificationAdModelBasedPredictor',
'CreativeNewTabPageAdModelBasedPredictor',
'CreativeInlineContentAdModelBasedPredictor',
],
},
param: [
{
name: 'last_seen_ad_predictor_weight',
value: '0.0',
},
{
name: 'last_seen_advertiser_predictor_weight',
value: '0.0',
},
],
},
],
filter: {
max_version: '128.1.69.51',
channel: [
'NIGHTLY',
'BETA',
'RELEASE',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
'ANDROID',
'IOS',
],
},
},
]
44 changes: 44 additions & 0 deletions studies/BraveAdsAdEventStudy.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
[
{
name: 'BraveAdsAdEventStudy',
experiment: [
{
name: 'Enabled',
probability_weight: 100,
feature_association: {
enable_feature: [
'AdEvent',
],
},
param: [
{
name: 'debounce_clicked_ad_event_for',
value: '1s',
},
{
name: 'deduplicate_clicked_ad_event_for',
value: '1s',
},
],
},
{
name: 'Default',
probability_weight: 0,
},
],
filter: {
channel: [
'NIGHTLY',
'BETA',
'RELEASE',
],
platform: [
'WINDOWS',
'MAC',
'LINUX',
'ANDROID',
'IOS',
],
},
},
]
Loading

0 comments on commit 9dc1c13

Please sign in to comment.