From 7833a2648f9ffbe6117b980d74cebebc0861797f Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Tue, 4 Feb 2025 13:51:47 -0800 Subject: [PATCH 1/2] Limit GLB to allowed IPs, automatically update if needed (#54242) --- .../alert-changed-branch-protections.yml | 2 +- .github/workflows/moda-allowed-ips.yml | 59 +++++++++++++++++++ .../production/services/webapp.yaml | 7 +-- src/workflows/tests/actions-workflows.ts | 2 +- 4 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/moda-allowed-ips.yml diff --git a/.github/workflows/alert-changed-branch-protections.yml b/.github/workflows/alert-changed-branch-protections.yml index 7ce9e2df8c8f..0c6b9754f784 100644 --- a/.github/workflows/alert-changed-branch-protections.yml +++ b/.github/workflows/alert-changed-branch-protections.yml @@ -4,7 +4,7 @@ on: branch_protection_rule: workflow_dispatch: schedule: - - cron: '20 16 * * 3' # Run every Wednesday at 16:30 UTC / 8:30 PST + - cron: '20 16 * * 3' # Run every Wednesday at 16:20 UTC / 8:20 PST permissions: contents: read diff --git a/.github/workflows/moda-allowed-ips.yml b/.github/workflows/moda-allowed-ips.yml new file mode 100644 index 000000000000..942bc4465bb1 --- /dev/null +++ b/.github/workflows/moda-allowed-ips.yml @@ -0,0 +1,59 @@ +name: Update Moda allowed IPs + +# **What it does**: Make sure that the allowed IPs in Moda are up to date. +# **Why we have it**: The IP ranges from Fastly can change. +# **Who does it impact**: Docs engineering. + +on: + schedule: + - cron: '20 16 * * 4' # Run every Thursday at 16:20 UTC / 8:20 PST + workflow_dispatch: + +permissions: + contents: write + pull-requests: write + +jobs: + update-moda-allowed-ips: + if: github.repository == 'github/docs-internal' + runs-on: ubuntu-latest + steps: + - name: Check out the repository + uses: actions/checkout@v4 + + - name: Update list of allowed IPs + run: | + echo "Getting a list of Fastly IP addresses...." + ips=$( \ + curl -s https://api.fastly.com/public-ip-list \ + | jq -r '.addresses | join(",")' \ + ) + echo "Got a list of Fastly IP addresses: $ips" + + echo "Updating the list of allowed IPs in Moda config..." + yq -i ".metadata.annotations[\"moda.github.net/allowed-ips\"] = \"$ips\"" \ + config/kubernetes/production/services/webapp.yaml + echo "Updated the list of allowed IPs in Moda config" + + echo "Checking if there is a change to make..." + if git diff --quiet; then + echo "No changes to the allowed IPs" + exit 0 + fi + + echo "Change found; making a pull request..." + branchname=update-allowed-ips-$(date +%s) + git checkout -b $branchname + git commit -am "Update list of allowed IPs" + git push + gh pr create \ + --title "Update list of allowed IPs" \ + --body 'This PR updates the list of allowed IPs in Moda. It is automatically generated.' \ + --head=$branchname + echo "Pull request created" + + - uses: ./.github/actions/slack-alert + if: ${{ failure() }} + with: + slack_channel_id: ${{ secrets.DOCS_ALERTS_SLACK_CHANNEL_ID }} + slack_token: ${{ secrets.SLACK_DOCS_BOT_TOKEN }} diff --git a/config/kubernetes/production/services/webapp.yaml b/config/kubernetes/production/services/webapp.yaml index 4c96ca5be8b5..bdb21c6e4ce8 100644 --- a/config/kubernetes/production/services/webapp.yaml +++ b/config/kubernetes/production/services/webapp.yaml @@ -7,11 +7,8 @@ metadata: annotations: moda.github.net/domain-name: 'docs-internal.github.com' moda.github.net/dns-registration-enabled: 'false' - moda.github.net/load-balancer-type: - public-external-http - # moda.github.net/allowed-ips: '23.235.32.0/20,43.249.72.0/22,103.244.50.0/24,103.245.222.0/23,103.245.224.0/24,104.156.80.0/20,140.248.64.0/18,140.248.128.0/17,146.75.0.0/17,151.101.0.0/16,157.52.64.0/18,167.82.0.0/17,167.82.128.0/20,167.82.160.0/20,167.82.224.0/20,172.111.64.0/18,185.31.16.0/22,199.27.72.0/21,199.232.0.0/1' - # ipv6 addresses not included - # curl -i "https://api.fastly.com/public-ip-list" + moda.github.net/load-balancer-type: public-external-http + moda.github.net/allowed-ips: 23.235.32.0/20,43.249.72.0/22,103.244.50.0/24,103.245.222.0/23,103.245.224.0/24,104.156.80.0/20,140.248.64.0/18,140.248.128.0/17,146.75.0.0/17,151.101.0.0/16,157.52.64.0/18,167.82.0.0/17,167.82.128.0/20,167.82.160.0/20,167.82.224.0/20,172.111.64.0/18,185.31.16.0/22,199.27.72.0/21,199.232.0.0/16 spec: ports: - name: http diff --git a/src/workflows/tests/actions-workflows.ts b/src/workflows/tests/actions-workflows.ts index ed30456c6d84..c4ee30d4d94a 100644 --- a/src/workflows/tests/actions-workflows.ts +++ b/src/workflows/tests/actions-workflows.ts @@ -10,7 +10,7 @@ import { chain, get } from 'lodash-es' const githubOwnedActionsRegex = /^(actions\/(cache|checkout|download-artifact|upload-artifact)@v\d+(\.\d+)*)$/ const actionHashRegexp = /^[A-Za-z0-9-/]+@[0-9a-f]{40}$/ -const checkoutRegexp = /^[actions/checkout]+@[0-9a-f]{40}$/ +const checkoutRegexp = /^[actions/checkout]+@(v\d+(\.\d+)*|[0-9a-f]{40})$/ const permissionsRegexp = /(read|write)/ type WorkflowMeta = { From a8a29c834c3d859b88b6cf5a01e4706714d17d74 Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Tue, 4 Feb 2025 16:05:08 -0800 Subject: [PATCH 2/2] add experiments pattern (#54228) Co-authored-by: Kevin Heis --- src/events/components/events.ts | 3 + src/events/components/experiment.ts | 34 ---- src/events/components/experiments/README.md | 172 ++++++++++++++++++ .../experiments/experiment-event.ts | 13 ++ .../components/experiments/experiment.ts | 153 ++++++++++++++++ .../components/experiments/experiments.ts | 39 ++++ .../experiments/useShouldShowExperiment.ts | 31 ++++ src/events/lib/schema.ts | 10 + src/events/middleware.ts | 2 + src/events/types.ts | 1 + src/fixtures/helpers/turn-off-experiments.ts | 46 +++++ src/fixtures/tests/playwright-a11y.spec.ts | 16 +- .../tests/playwright-local-dev.spec.ts | 7 +- .../tests/playwright-rendering.spec.ts | 49 ++++- src/frame/pages/app.tsx | 6 +- 15 files changed, 539 insertions(+), 43 deletions(-) delete mode 100644 src/events/components/experiment.ts create mode 100644 src/events/components/experiments/README.md create mode 100644 src/events/components/experiments/experiment-event.ts create mode 100644 src/events/components/experiments/experiment.ts create mode 100644 src/events/components/experiments/experiments.ts create mode 100644 src/events/components/experiments/useShouldShowExperiment.ts create mode 100644 src/fixtures/helpers/turn-off-experiments.ts diff --git a/src/events/components/events.ts b/src/events/components/events.ts index b770041ec004..39996f6cd828 100644 --- a/src/events/components/events.ts +++ b/src/events/components/events.ts @@ -3,6 +3,7 @@ import Cookies from 'src/frame/components/lib/cookies' import { parseUserAgent } from './user-agent' import { Router } from 'next/router' import { isLoggedIn } from 'src/frame/components/hooks/useHasAccount' +import { getExperimentVariationForContext } from './experiments/experiment' import { EventType, EventPropsByType } from '../types' const COOKIE_NAME = '_docs-events' @@ -110,6 +111,8 @@ export function sendEvent({ color_mode_preference: getColorModePreference(), os_preference: Cookies.get('osPreferred'), code_display_preference: Cookies.get('annotate-mode'), + + experiment_variation: getExperimentVariationForContext(getMetaContent('path-language')), }, ...props, diff --git a/src/events/components/experiment.ts b/src/events/components/experiment.ts deleted file mode 100644 index 785931ce3155..000000000000 --- a/src/events/components/experiment.ts +++ /dev/null @@ -1,34 +0,0 @@ -import murmur from 'imurmurhash' -import { getUserEventsId, sendEvent } from './events' -import { EventType } from '../types' - -let initialized = false - -const TREATMENT = 'TREATMENT' -const CONTROL = 'CONTROL' - -export function bucket(test: string) { - const id = getUserEventsId() - const hash = murmur(test).hash(id).result() - return hash % 2 ? TREATMENT : CONTROL -} - -export function sendSuccess(test: string) { - return sendEvent({ - type: EventType.experiment, - experiment_name: test, - experiment_variation: bucket(test).toLowerCase(), - experiment_success: true, - }) -} - -export function initializeExperiments() { - if (initialized) return - initialized = true - // *** Example test code *** - // const testName = '$test-name$' - // const xbucket = bucket(testName) - // const x = document.querySelector(...) - // x.addEventListener('click', () => { sendSuccess(testName) }) - // if (xbucket === TREATMENT) applyTreatment(x) -} diff --git a/src/events/components/experiments/README.md b/src/events/components/experiments/README.md new file mode 100644 index 000000000000..eee3eca8171f --- /dev/null +++ b/src/events/components/experiments/README.md @@ -0,0 +1,172 @@ +# Experiments + +There are times when we want make a change, but aren't sure if it will provide a better user experience. + +In these scenarios we can run experiments. + +Experiments are A/B tests where A is some version of our site and B is another. When the user requests our site they are randomly served either site A or B. + +After the experiment is live, we gather data via [events](../../README.md) that help determine which version of the site we want to stick with. + +## TOC + +- [Experiments as feature flags](#experiments-as-feature-flags) +- [Experiment variations](#experiment-variations) +- [Adding an experiment](#adding-an-experiment) +- [Implementing an experiment](#implementing-an-experiment) +- [Toggling an experiment on or off](#toggling-an-experiment-on-or-off) +- [Tracking results on an experiment](#tracking-results-on-an-experiment) + - [Via regular events](#via-regular-events) + - [Via the `experiment` event](#via-the-experiment-event) + + +## Experiments as feature flags + +An additional benefit of this pattern is that it lets you turn on/off a feature in the UI and toggle it from the developer console. This is useful if you want to ship UI changes in parts, or test something in production before turning it on. + +## Experiment variations + +To clarify terminology, if a user is shown site A which is the original site _without_ the experiment they will have an `experiment_variation` value of `"control"` to indicate that they are in the control group. + +If the user is shown the experiment (site B), they will have an `experiment_variation` value of `"treatment"` + +## Adding an experiment + +1. Create a `key` for you experiment, e.g. `ai_search_experiment` +2. Determine how many users will see the experiment. The default is 50% and makes sense for _most_ use cases. +3. Add your experiment to [experiments.ts](./experiments.ts) + +Example, + +```typescript +// Add new experiment key to this list +export type ExperimentNames = 'example_experiment' | 'ai_search_experiment' + +export const EXPERIMENTS = { + example_experiment: { ... } + ai_search_experiment: { + key: 'ai_search_experiment', + isActive: true, // Set to false when the experiment is over + percentOfUsersToGetExperiment: 10, // Only 10% of users will get the experiment + limitToLanguages: ['en'], // Only users with the `en` language will be included in the experiment + includeVariationInContext: true, // See note below + }, +} +``` + +When `includeVariationInContext` is true **all** analytics events sent will include `experiment_variation` in their context. `experiment_variation` will be `"treatment"` or `"control"` depending on which the user was randomly assigned. + +> [!IMPORTANT] +> Since the `experiment_variation` is a single key in the context, **only one experiment** can include their variations in the context e.g. only one value of `includeVariationInContext` can be `true`. + +## Implementing an experiment + +For example, let's say you are conducting an experiment that changes the search bar. + +In the code that displays the search bar, you can use the `shouldShowExperiment` function to determine which version of the code to show the user. + +Example: + +```typescript +import { useRouter } from 'next/router' +import { useShouldShowExperiment } from '@/events/components/experiments/useShouldShowExperiment' +import { EXPERIMENTS } from '@/events/components/experiments/experiments' +import { ClassicSearchBar } from "@/search/components/ClassicSearchBar.tsx" +import { NewSearchBar } from "@/search/components/NewSearchBar.tsx" + +export function SearchBar() { + const router = useRouter() + // Users who were randomly placed in the `treatment` group will be shown the experiment + const { shouldShow: shouldShowNewSearch } = useShouldShowExperiment( + EXPERIMENTS.ai_search_experiment, + router.locale + ) + + if (shouldShowNewSearch) { + return ( + + ) + } + return +} +``` + +> [!NOTE] +> If a user is placed in the `"treatment"` group e.g. they are shown the experiment and will continue to see the treatment across all sessions from the same browser. This is because we use a hash of user's session ID cookie to deterministically set the control group. The session cookie lasts for 365 days, otherwise they might see something different on each reload. + +## Toggling an experiment on or off + +In development every session is placed into the `"treatment"` control group so that the experiment can be developed on. + +However, you can change which experiment to show by calling the following function in the `Console` tab in Chrome dev tools, + +```javascript +window.overrideControlGroup("", "treatment" | "control"); + +// Example to see original search experience +window.overrideControlGroup("ai_search_experiment", "control"); +``` + +For events, you can verify that your `experiment_variation` values are being included in the event context from the `Network` tab in Chrome dev tools. + +## Tracking results on an experiment + +### Via regular events + +If your experiment object in [experiments.ts](./experiments.ts) included the `includeVariationInContext: true` key (and is the ONLY object to include that key) then the `experiment_variation` of your experiment will be sent in the context of an event. + +This means that you can send other events, like + +```typescript +sendEvent({ + type: EventType.search, + search_query: "How do I open pdf?", + search_context: "general-search", +}); +``` + +And the `context` on that event will include the `experiment_variation` key and value of your experiment, + +e.g. + +```javascript +{ + search_query: "How do I open pdf?", + search_context: "general-search", + context: { + ... + experiment_variation: "treatment" // Could also be "control" depending on the random outcome + } +} +``` + +### Via the `experiment` event + +If your experiment is specific, meaning it can be tracked with a boolean event, e.g. + +```javascript +{ + experiment_name: , // e.g. `new_button_experiment` for did user click new button? + experiment_variation: 'treatment' | 'control', + experiment_success: , // e.g. true the user is using the new button! +} +``` + +Then you should omit the `includeVariationInContext` key from your experiment object and use the `sendExperimentSuccess` function to track events. + +Example: + +```typescript +import { sendExperimentSuccess } from '@/events/components/experiments/experiment-event' +import { EXPERIMENTS } from '@/events/components/experiments/experiments' + +export function MyNewComponent() { + return ( +