Stop setting statsig_stable_id as cookie #65673

nicklathe · 2025-05-05T22:54:14Z

📝 PR Overview

This PR updates how we capture signed-out user metrics in order to align with our cookie policy and fix a OneTrust integration issue.

🔍 Background

Previously, we used a statsig_stable_id cookie to uniquely identify user sessions for Statsig. This allowed Statsig to stitch anonymous events (signed-out users) and associate them with a signed-in user later.

However:

This broke OneTrust functionality (Jira ticket)
It did not comply with our cookie policy, which requires user consent for cookies we set

✅ What this PR changes

To address the above, we now:

Store the statsig_stable_id in the server-side session (instead of a cookie)
Pass the stable ID to the frontend via a <script> tag, following established conventions

This approach preserves the ability to capture anonymous user metrics without violating cookie restrictions or breaking OneTrust.

Links

Jira https://codedotorg.atlassian.net/browse/P20-1505
OneTrust Jira: https://codedotorg.atlassian.net/browse/P20-1454

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

Tests provide adequate coverage
Privacy and Security impacts have been assessed
Code is well-commented
New features are translatable or updates will not break translations
Relevant documentation has been added or updated
User impact is well-understood and desirable
Pull Request is labeled appropriately
Follow-up work items (including potential tech debt) are tracked and linked

dashboard/app/controllers/application_controller.rb

apps/src/metrics/statsigHelpers.js

nicklathe · 2025-05-06T20:51:11Z

dashboard/app/controllers/lti/v1/account_linking_controller.rb

@@ -51,6 +51,7 @@ def link_email
            user: existing_user,
            event_name: 'lti_user_signin',
            metadata: metadata,
+            session: session,


Passing in session here to extract the statsig_stable_id. We are not including a stable ID in a lot of the LTI related event logs, however given that this functionality links a user who launched from an LMS to an existing user account, I think it could be helpful to include a stable ID in the log. Or maybe not 🤷‍♂️

Did you check to make sure there isn't a security or privacy issue with logging the whole session? How big is the session? I think we were having issues for a long time because we were storing too much data in Cookies, so we moved it to the session. Now I'm concerned we are dumping a lot more data into this event. I think it's a little risky for "may be useful". If there is a lot of data, this code could instead be specific about which data from the session is being logged.

I think my comment is confusing, I've re-worded it.

It doesn't log the session. We persist the statsig_stable_id in session (in the application_controller.rb above), then inside of Metrics::Events.log_event, we dig it out of the session (link to below). In order to access session from a non-controller (I.e. the Metrics::Events module), we have to pass in session as it doesn't have access to it.

Basically what this functionality does is, if you want to include the stable ID in the event you're logging, you pass in the session to the log_event method, and it will extract it. If you don't care about including a stable ID, you can chose to not pass in the session. There are a lot of places where were weren't including the stable ID in the event, because we already have a signed-in user (therefore a user ID). However this account linking step is the case where a user launches from an LMS, and they can chose to continue with creating a new user or link to an existing account. I figured it could be helpful to include the stable ID in this step, which is why I added it.

daynew

Looks good, although I have one blocking concern related to logging the whole session.

daynew · 2025-05-06T22:24:37Z

apps/src/metrics/statsigHelpers.js

@@ -13,15 +11,8 @@ export function getUserType() {
 }

 export function findOrCreateStableId() {


nit This doesn't "Create" a stable ID anymore.

Good call. Actually now I'm wondering if I should have it create a stable ID in the case it doesn't get one passed to it from the backend 🤔

Well, except that we wouldn't have anywhere to store it if we created one here (can't use a cookie like before). This would run on each page load and generate a new ID each time, which would not be very "stable". I think I'll just change the name and expect that it will be passed from the backend.

daynew · 2025-05-06T22:52:03Z

dashboard/app/controllers/lti/v1/account_linking_controller.rb

@@ -51,6 +51,7 @@ def link_email
            user: existing_user,
            event_name: 'lti_user_signin',
            metadata: metadata,
+            session: session,


Did you check to make sure there isn't a security or privacy issue with logging the whole session? How big is the session? I think we were having issues for a long time because we were storing too much data in Cookies, so we moved it to the session. Now I'm concerned we are dumping a lot more data into this event. I think it's a little risky for "may be useful". If there is a lot of data, this code could instead be specific about which data from the session is being logged.

- Set statsig_stable_id in session - Pass id to frontend via script tag functionality Signed-off-by: Nick Lathe <[email protected]>

- Session was added to Metrcis::Events.log_event in places that are unecessary, due to having a signed_in user

- .dig for statsig_stable_id from session with safe navigation operator - Remove unecessary line that clears cookies during logout

shared/middleware/varnish_environment.rb

nicklathe commented May 5, 2025

View reviewed changes

dashboard/app/controllers/application_controller.rb Show resolved Hide resolved

nicklathe commented May 6, 2025

View reviewed changes

apps/src/metrics/statsigHelpers.js Outdated Show resolved Hide resolved

nicklathe commented May 6, 2025

View reviewed changes

nicklathe marked this pull request as ready for review May 6, 2025 21:05

daynew approved these changes May 6, 2025

View reviewed changes

nicklathe added 4 commits May 7, 2025 12:32

Stop setting statsig_stable_id as cookie

570bccf

- Set statsig_stable_id in session - Pass id to frontend via script tag functionality Signed-off-by: Nick Lathe <[email protected]>

Remove session that was added to controllers

33b7938

- Session was added to Metrcis::Events.log_event in places that are unecessary, due to having a signed_in user

Improve readability of findOrCreateStableId function

e255f78

Correctly access statsig_stable_id from session

5d981b9

- .dig for statsig_stable_id from session with safe navigation operator - Remove unecessary line that clears cookies during logout

nicklathe force-pushed the P20-1505/remove-statsig-stable-id-cookie branch from 16e4995 to 5d981b9 Compare May 7, 2025 19:33

nicklathe added 2 commits May 7, 2025 15:15

Change 'request' to 'session' in log_event method

2ffbf84

Include missed file from last commit

3d211ce

nicklathe commented May 7, 2025

View reviewed changes

shared/middleware/varnish_environment.rb Outdated Show resolved Hide resolved

nicklathe added 3 commits May 7, 2025 15:51

Change function name and simplify log_event arg

03ab253

Change straggling 'request' to 'session'

9effd69

Change request -> session in global_edition_test.rb

1f57898

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop setting statsig_stable_id as cookie #65673

Stop setting statsig_stable_id as cookie #65673

nicklathe commented May 5, 2025 •

edited

Loading

nicklathe May 6, 2025 •

edited

Loading

daynew May 6, 2025

nicklathe May 7, 2025 •

edited

Loading

nicklathe May 7, 2025

daynew left a comment

daynew May 6, 2025

nicklathe May 7, 2025

nicklathe May 7, 2025 •

edited

Loading

daynew May 6, 2025

		@@ -13,15 +11,8 @@ export function getUserType() {
		}

		export function findOrCreateStableId() {

Stop setting statsig_stable_id as cookie #65673

Are you sure you want to change the base?

Stop setting statsig_stable_id as cookie #65673

Conversation

nicklathe commented May 5, 2025 • edited Loading

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

nicklathe May 6, 2025 • edited Loading

Choose a reason for hiding this comment

daynew May 6, 2025

Choose a reason for hiding this comment

nicklathe May 7, 2025 • edited Loading

Choose a reason for hiding this comment

nicklathe May 7, 2025

Choose a reason for hiding this comment

daynew left a comment

Choose a reason for hiding this comment

daynew May 6, 2025

Choose a reason for hiding this comment

nicklathe May 7, 2025

Choose a reason for hiding this comment

nicklathe May 7, 2025 • edited Loading

Choose a reason for hiding this comment

daynew May 6, 2025

Choose a reason for hiding this comment

nicklathe commented May 5, 2025 •

edited

Loading

nicklathe May 6, 2025 •

edited

Loading

nicklathe May 7, 2025 •

edited

Loading

nicklathe May 7, 2025 •

edited

Loading