Initial Amplitude Implementation, second try #49442
Conversation
…, and format userId based on environment
| @@ -13,3 +13,6 @@ custom_error_response: false | |||
|
|
|||
| # Configurations for making calls to the Immersive Reader API on Azure | |||
| imm_reader_client_id: '74937af0-55ee-411a-867a-57eefadec7d9' | |||
|
|
|||
| # General CDO Amplitude key | |||
| cdo_amplitude_api_key: !Secret | |||
There was a problem hiding this comment.
I think this is fine but I'm confused still why staging broke before. It's definitely possible y'all understand and I'm still playing catchup so if that's the case, feel free to merge :)
There was a problem hiding this comment.
I definitely don't understand why it broke, so if you have any other ideas we should try those too!
| @@ -23,6 +23,8 @@ | |||
| -# data needed for dcdo.js. This tag is needed on every page because our translation code is | |||
| -# making use of the dcdo feature. | |||
| %script{data: {'dcdo': DCDO.frontend_config.to_json}} | |||
| -# auth key needed for amplitude analytics. | |||
| %script{data: {'amplitude-api-key': CDO.safe_amplitude_api_key}} | |||
There was a problem hiding this comment.
It's worth noting that if the key can't be found, <script data-amplitude-api-key=""></script> will be on the page but if the key is nil this element won't be on the page at all. This seems fine though because we can handle the element not existing
There was a problem hiding this comment.
Yeah, the JS handles both cases in basically the same way, but I do prefer the empty string one because it makes it easier to look for the script tag in the page during development. That tripped me up a few times looking for an attribute that didn't exist.
This reverts commit 8b6fbcf.
Followup to #49374/#49440 , with modified config.
Links
Testing story
Must be tested after deployment, unfortunately.
Deployment strategy
Secrets already added to AWS.