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

[Analytics] Prepare for Mixpanel Migration of website Insights #8354

Merged
merged 2 commits into from
Mar 2, 2022

Conversation

jakobhero
Copy link
Contributor

@jakobhero jakobhero commented Feb 21, 2022

Description

PR prepares to move visitor (i.e. everyone but authenticated user) data out of mixpanel through the following two steps:

  • track the qualification (whether user has previously visited the website) at signup. as it will not be possible to compute this in mixpanel anymore by checking if a website visit was tracked from the user's profile previously, this is now passed to the app through a cookie and inside the payload of the signup event
  • extend the implementation of the SegmentAnalyticsWriter to feature the integrations object in the payload, which ensures that an event is not forwarded to Mixpanel if the user is not authenticated. This has to be specified in the app as the event filtering in Segment unfortunately does not allow to check for the existence of userId

These two changes are prerequisites to converting to a paying subscription with Mixpanel as we would else either risk data loss or a spike in the cost of ownership.

Related Issue(s)

How to test

  • open preview environment and Segment's Staging Untrusted
  • Check in the raw tab of the first page() call whether integrations exists with "Mixpanel": false (the initial page call should occur before authenticating and thus not be forwarded to Mixpanel as the userId is not passed along)
  • sign up
  • Check in the raw tab of the signup track event sent to Segment that integrations exists with "Mixpanel": true (signups always contain the userId, so Segment should forward this to Mixpanel). Then, check that the signup event contains a qualified property with the value false (as the cookie gitpod-marketing-website-visited should not yet exist for this domain)
  • Next, open a different browser or incognito window, and open the preview environment once again. Then, set the cookie gitpod-marketing-website-visited=true (current domain, expiry at least session; one way to do this is to go to the console of the developer tool and enter document.cookie="gitpod-marketing-website-visited=true")
  • Now, sign up with a different account to trigger another signup event call and make sure that the qualified property in the Segment Debugger is true this time

Release Notes

NONE

Documentation

/werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@jakobhero
Copy link
Contributor Author

jakobhero commented Feb 21, 2022

/werft run

👍 started the job as gitpod-build-mixpanel-migration-prep.1

@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #8354 (c84872f) into main (e2d8e2d) will increase coverage by 2.75%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            main    #8354      +/-   ##
=========================================
+ Coverage   8.42%   11.17%   +2.75%     
=========================================
  Files         33       18      -15     
  Lines       2339      993    -1346     
=========================================
- Hits         197      111      -86     
+ Misses      2137      880    -1257     
+ Partials       5        2       -3     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-installer-raw-app ?
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...staller/pkg/components/ws-manager/networkpolicy.go
...components/ws-manager/unpriviledged-rolebinding.go
...s/installer/pkg/components/ws-manager/configmap.go
components/local-app/pkg/auth/pkce.go
...onents/installer/pkg/components/ws-manager/role.go
components/installer/pkg/common/storage.go
components/local-app/pkg/auth/auth.go
.../installer/pkg/components/ws-manager/deployment.go
components/installer/pkg/common/render.go
components/installer/pkg/common/display.go
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2d8e2d...c84872f. Read the comment docs.

@jakobhero jakobhero marked this pull request as ready for review February 21, 2022 16:55
@jakobhero jakobhero requested a review from a team February 21, 2022 16:55
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Feb 21, 2022
@geropl
Copy link
Member

geropl commented Mar 2, 2022

/werft run

👍 started the job as gitpod-build-mixpanel-migration-prep.2

@geropl
Copy link
Member

geropl commented Mar 2, 2022

Starting review now...

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

LGTM, tested and works as advertised

@roboquat roboquat merged commit 9880270 into main Mar 2, 2022
@roboquat roboquat deleted the mixpanel-migration-prep branch March 2, 2022 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants