Skip to content
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

GDP-1766: Add Amplitude #19

Merged
merged 1 commit into from
Oct 4, 2023
Merged

GDP-1766: Add Amplitude #19

merged 1 commit into from
Oct 4, 2023

Conversation

danielcmessias
Copy link

@danielcmessias danielcmessias commented Aug 23, 2023

GDP-1766

TLDR; trying to use environment variables in web apps is a pain in the...

You would not believe how long I spent trying to set these API keys properly. I'm certain there's a better way, but I'll be damned if I could figure it out. If anybody wants to take over this and try to find a better solution, please do, but I am done with it! This works 😅

Changes:

  • Adds additional identify call to better segment users
  • Add # of Data Products figure to the Analytics page
  • Change the Amplitude SDK config to point to EU servers
  • seds the Amplitude keys into the codebase
  • Builds the DataHub frontend Docker image with Drone and push to ECR (separate images because the API keys are hardcoded 🙈 ). PR in fc-datahub that points to these images

Tested in eUat/Staging. Events are getting sent to Amplitude 🥳


These API keys are not secret values. Think of them more like integration keys. We are ok :)


Screenshot 2023-10-03 at 13 07 07 Screenshot 2023-10-03 at 13 08 40

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@danielcmessias danielcmessias changed the title Gdp 1766 amplitude GDP-1766: Add Amplitude Oct 2, 2023
@danielcmessias danielcmessias marked this pull request as ready for review October 3, 2023 12:08
Copy link
Collaborator

@harsha-mandadi-4026 harsha-mandadi-4026 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@danielcmessias danielcmessias merged commit f386b3f into master Oct 4, 2023
38 of 44 checks passed
@danielcmessias danielcmessias deleted the GDP-1766_amplitude branch October 4, 2023 09:19
@danielcmessias danielcmessias restored the GDP-1766_amplitude branch October 6, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants