-
Notifications
You must be signed in to change notification settings - Fork 0
Ftr/users lcp #59
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
Open
jgoddard19
wants to merge
11
commits into
main
Choose a base branch
from
ftr/users_lcp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Ftr/users lcp #59
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
66318fa
feat: first pass of the scenario! changes scenario service to add new…
jgoddard19 20cf32a
feat: working tests
jgoddard19 d2b247a
fix: remove duplicate /scenario-runner path in test workflow causing …
jgoddard19 9972aa1
corrected URLs
jgoddard19 bbe3773
fix: add environment to test-summary job for BASE_URL access
jgoddard19 4db867e
fix: update browser user test for new A/B testing behavior
jgoddard19 b9593b6
fix: use unique UUIDs for cohort distribution test
jgoddard19 461f4ea
fix: use unique UUIDs in 0% and 100% assignment tests
jgoddard19 80bb2cd
fix: run A/B test scenarios sequentially to avoid race conditions
jgoddard19 b5ab9c5
docs: add A/B testing scenarios to test suite documentation
jgoddard19 cdb3aae
feat: add LCP A/B testing with percentage-based and cohort-based scen…
jgoddard19 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| name: Flow - LCP A/B Test Scenario | ||
|
|
||
| on: | ||
| schedule: | ||
| # Run every 6 hours | ||
| - cron: '0 */6 * * *' | ||
| workflow_dispatch: # Allow manual trigger | ||
|
|
||
| jobs: | ||
| lcp-ab-test: | ||
| runs-on: ubuntu-latest | ||
| environment: events | ||
|
|
||
| steps: | ||
| - name: Enable LCP slowness for 50% of users | ||
| run: | | ||
| echo "Enabling LCP slowness A/B test (50% of users, 3000ms delay)..." | ||
| curl -X POST "${{ vars.BASE_URL }}/scenario-runner/api/ab-testing/lcp-slowness?enabled=true&percentage=50.0&delay_ms=3000" \ | ||
| --fail-with-body || { | ||
| echo "Failed to enable LCP slowness scenario" | ||
| exit 1 | ||
| } | ||
|
|
||
| - name: Verify scenario is enabled | ||
| run: | | ||
| echo "Verifying LCP slowness scenario configuration..." | ||
| response=$(curl -s "${{ vars.BASE_URL }}/scenario-runner/api/ab-testing/config") | ||
| echo "Response: $response" | ||
|
|
||
| enabled=$(echo $response | jq -r '.config.lcp_slowness_enabled') | ||
| percentage=$(echo $response | jq -r '.config.lcp_slowness_percentage') | ||
| delay=$(echo $response | jq -r '.config.lcp_slowness_delay_ms') | ||
|
|
||
| if [ "$enabled" != "true" ]; then | ||
| echo "ERROR: LCP slowness not enabled" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✓ LCP slowness enabled: $percentage% of users with ${delay}ms delay" | ||
|
|
||
| - name: Create deployment marker (A/B test enabled) | ||
| env: | ||
| NEW_RELIC_API_KEY: ${{ secrets.NEW_RELIC_API_KEY }} | ||
| NEW_RELIC_APP_ID: ${{ secrets.NEW_RELIC_BROWSER_APP_ID }} | ||
| run: | | ||
| echo "Creating deployment marker in New Relic..." | ||
| curl -X POST "https://api.newrelic.com/v2/applications/${NEW_RELIC_APP_ID}/deployments.json" \ | ||
| -H "Api-Key: ${NEW_RELIC_API_KEY}" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "deployment": { | ||
| "revision": "LCP-AB-Test-Enabled-50%", | ||
| "description": "Feature flag: LCP slowness enabled for 50% of users (3000ms delay)", | ||
| "user": "GitHub Actions", | ||
| "changelog": "A/B test started to measure LCP impact" | ||
| } | ||
| }' || echo "Warning: Could not create deployment marker" | ||
|
|
||
| - name: Wait for data collection (20 minutes) | ||
| run: | | ||
| echo "Collecting data for 20 minutes..." | ||
| echo "Monitor LCP metrics at: https://one.newrelic.com/" | ||
| echo "" | ||
| echo "NRQL Query to see A/B test impact:" | ||
| echo "SELECT percentile(largestContentfulPaint, 50, 75, 95) as 'LCP (ms)', count(*) as 'Sessions'" | ||
| echo "FROM PageViewTiming" | ||
| echo "WHERE appName = 'ReliBank Frontend' AND pageUrl LIKE '%/dashboard%'" | ||
| echo "FACET custom.lcp_treatment TIMESERIES" | ||
| echo "" | ||
| sleep 1200 # 20 minutes | ||
|
|
||
| - name: Disable LCP slowness | ||
| run: | | ||
| echo "Disabling LCP slowness A/B test..." | ||
| curl -X POST "${{ vars.BASE_URL }}/scenario-runner/api/ab-testing/lcp-slowness?enabled=false" \ | ||
| --fail-with-body || { | ||
| echo "Warning: Failed to disable LCP slowness scenario" | ||
| } | ||
|
|
||
| - name: Verify scenario is disabled | ||
| run: | | ||
| echo "Verifying LCP slowness scenario is disabled..." | ||
| response=$(curl -s "${{ vars.BASE_URL }}/scenario-runner/api/ab-testing/config") | ||
| enabled=$(echo $response | jq -r '.config.lcp_slowness_enabled') | ||
|
|
||
| if [ "$enabled" == "true" ]; then | ||
| echo "WARNING: LCP slowness still enabled!" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✓ LCP slowness disabled successfully" | ||
|
|
||
| - name: Create rollback marker (A/B test disabled) | ||
| env: | ||
| NEW_RELIC_API_KEY: ${{ secrets.NEW_RELIC_API_KEY }} | ||
| NEW_RELIC_APP_ID: ${{ secrets.NEW_RELIC_BROWSER_APP_ID }} | ||
| run: | | ||
| echo "Creating rollback marker in New Relic..." | ||
| curl -X POST "https://api.newrelic.com/v2/applications/${NEW_RELIC_APP_ID}/deployments.json" \ | ||
| -H "Api-Key: ${NEW_RELIC_API_KEY}" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "deployment": { | ||
| "revision": "LCP-AB-Test-Disabled", | ||
| "description": "Rollback: LCP slowness disabled, all users back to normal", | ||
| "user": "GitHub Actions", | ||
| "changelog": "A/B test completed" | ||
| } | ||
| }' || echo "Warning: Could not create rollback marker" | ||
|
|
||
| - name: Test Summary | ||
| if: always() | ||
| run: | | ||
| echo "## LCP A/B Test Summary" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **Duration**: 20 minutes" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **Cohort Split**: 50% slow / 50% normal" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **LCP Delay**: 3000ms for slow cohort" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "### Analysis" >> $GITHUB_STEP_SUMMARY | ||
| echo "Review LCP metrics in New Relic to compare:" >> $GITHUB_STEP_SUMMARY | ||
| echo "- \`lcp_treatment = normal\` (control group)" >> $GITHUB_STEP_SUMMARY | ||
| echo "- \`lcp_treatment = slow\` (test group with 3s delay)" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "### NRQL Query" >> $GITHUB_STEP_SUMMARY | ||
| echo '```sql' >> $GITHUB_STEP_SUMMARY | ||
| echo 'SELECT percentile(largestContentfulPaint, 50, 75, 95) as "LCP (ms)", count(*) as "Sessions"' >> $GITHUB_STEP_SUMMARY | ||
| echo 'FROM PageViewTiming' >> $GITHUB_STEP_SUMMARY | ||
| echo "WHERE appName = 'ReliBank Frontend' AND pageUrl LIKE '%/dashboard%'" >> $GITHUB_STEP_SUMMARY | ||
| echo 'FACET custom.lcp_treatment' >> $GITHUB_STEP_SUMMARY | ||
| echo 'SINCE 30 minutes ago' >> $GITHUB_STEP_SUMMARY | ||
| echo '```' >> $GITHUB_STEP_SUMMARY | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,13 +119,16 @@ export function Layout({ children }: { children: React.ReactNode }) { | |
| if (typeof window !== 'undefined' && isHydrated) { | ||
| // Check sessionStorage first | ||
| const storedUserId = sessionStorage.getItem('browserUserId'); | ||
| if (storedUserId) { | ||
| const storedLcpDelay = sessionStorage.getItem('lcpDelayMs'); | ||
|
|
||
| if (storedUserId && storedLcpDelay !== null) { | ||
| console.log('[Browser User] Loaded from sessionStorage:', storedUserId); | ||
| console.log('[Browser User] LCP Delay from sessionStorage:', storedLcpDelay + 'ms'); | ||
| setBrowserUserId(storedUserId); | ||
| return; | ||
| } | ||
|
|
||
| // Fetch from API | ||
| // Fetch from API (either no user ID or no LCP delay stored) | ||
| const fetchBrowserUserId = async () => { | ||
| try { | ||
| const response = await fetch('/accounts-service/browser-user'); | ||
|
|
@@ -134,6 +137,17 @@ export function Layout({ children }: { children: React.ReactNode }) { | |
| console.log(`[Browser User] Received: ${data.user_id} Source: ${data.source}`); | ||
| setBrowserUserId(data.user_id); | ||
| sessionStorage.setItem('browserUserId', data.user_id); | ||
|
|
||
| // Store LCP delay for A/B testing | ||
| const lcpDelay = data.lcp_delay_ms || 0; | ||
| sessionStorage.setItem('lcpDelayMs', lcpDelay.toString()); | ||
| console.log(`[Browser User] LCP Delay: ${lcpDelay}ms`); | ||
|
|
||
| // Set New Relic custom attribute for A/B test cohort tracking | ||
| if (window.newrelic && typeof window.newrelic.setCustomAttribute === 'function') { | ||
| window.newrelic.setCustomAttribute('lcp_delay_ms', lcpDelay); | ||
| window.newrelic.setCustomAttribute('lcp_treatment', lcpDelay > 0 ? 'slow' : 'normal'); | ||
| } | ||
| } else { | ||
| console.error('[Browser User] Failed to fetch user ID:', response.status); | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Copilot Autofix
AI about 16 hours ago
In general, to fix this class of issue, you add a
permissionsblock either at the root of the workflow (to apply to all jobs) or inside specific jobs, and set the minimal scopes necessary. For a workflow that only calls external APIs and writes to$GITHUB_STEP_SUMMARY, you typically only needcontents: read(the minimal default for most workflows) and no write permissions.For this specific workflow in
.github/workflows/flow-lcp-ab-test.yml, the simplest and least invasive fix is to add a top-levelpermissionsblock right under thename:(and beforeon:). The job does not perform any GitHub write actions (noactions/checkout, no pushes, no issue/PR operations), so we can safely lock the token down tocontents: read. No additional imports or steps are required; this change only affects token scopes. Concretely, in the region around lines 1–8, add:and keep the rest of the workflow unchanged.