Updates User Configuration and Turns off Statsig auto js error logging #57481
Conversation
apps/src/lib/util/StatsigReporter.js
Outdated
| @@ -12,6 +12,17 @@ const NO_EVENT_NAME = 'NO_VALID_EVENT_NAME_LOG_ERROR'; | |||
|
|
|||
| class StatsigReporter { | |||
| constructor() { | |||
| let user = {}; | |||
| user_id_element = document.querySelector('script[data-user-id]'); | |||
There was a problem hiding this comment.
We'll want to be careful about relying on this on cached pages, where it'll become out of date. The nice thing about using userRedux is that it's updated for every page load so it was always (eventually) up to date.
I'm curious if we can still use the user passed through from userRedux, but save that user id in this class so that we can send it with events? Or is there a race condition there that would make that difficult?
There was a problem hiding this comment.
Can we pair on this? It seems that the current user redux has been updating Amplitude on init(), which doesn't seem to get called for each page load.
I'm also surprised that a page would be cached through a change in user log out/sign in. This would make me think that some users could access data from a previously logged in user's cache. Is that the case or am I misunderstanding?
There was a problem hiding this comment.
oh weird -- it's probably not great if it doesn't get called on each page load 😬
On cached pages (and also I think on uncached pages as a result), we fetch the user data asyncronously, which then is fed into the redux store here. Most of our pages aren't cached, so in most cases, this solution is fine, but it's always possible we accidentally try to use this on one of those cached page and become confused.
|
Also can you update the title of this PR? It seems like this is a bit more than turning off JS error logging now :) |
hannahbergam
changed the title
Ah yes- good call! Done |
| @@ -6,6 +6,7 @@ | |||
| = render partial: 'i18n/crowdin_in_context_tool' | |||
| = render inline: File.read(Rails.root.join('..', 'shared', 'haml', 'onetrust_cookie_scripts.haml')), type: :haml, locals: {dashboard: true, domain: 'code.org'} | |||
| - appname = Rails.env.production? ? t(:appname) : "#{t(:appname)} [#{Rails.env}]" | |||
| - managed_test_server = "#{CDO.running_web_application? && CDO.test_system?}" | |||
There was a problem hiding this comment.
did you ever figure out why this was set wrong? (can totally be done in a separate PR if not)
There was a problem hiding this comment.
I don't think I was injecting ruby in the correct way before so am abstracting it out into its own variable to match what else is happening in this file!
When Statsig is initialized, it defaults to logging all js errors. This means that now our integration is working and we're logging lots of errors! The options were being logged to the user object. This pointed to a bigger issue with the way we were planning to keep track of user data. On Amplitude, we send it through Current User Redux and that persists across a user's session. For Statsig, they strongly recommend sending the user in during the initialize() call and 'do not maintain or update user objects'. That led to this new solution:
This way, the options parameter will always be the third parameter so flags like disableErrorLogging will work properly. A bonus: this now more closely matches our Statsig ruby implementation, where the user is sent in on initialize.
Additionally, I am updating the managed_test_server param to its own variable before sending it through, which I believe will enable our test server to send events.
These errors do not count toward our billable events (see images from slack conversation with Statsig directly below).
Links
Testing story
I was able to show that initializing with user data makes it so every page refresh has a user_id, not just when we have a login (and the single send from current user redux). See image below.
I was also able to verify that pegasus reporting still works as anticipated (I updated the amplitude event for 'page viewed' to send to statsig and it correctly sent with an empty user object). See second image below!
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: