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

chore: [M3-6497] - Remove Google Analytics and clean up custom events #9266

Merged

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Jun 15, 2023

Description 📝

Final Google Analytics -> Adobe Analytics migration PR with the clean up to remove GA from Cloud Manager. We're finally deprecating GA in our 6/26 release!

TODO:

  • Merge corresponding internal PR for removing REACT_APP_GA_ID and REACT_APP_GTM_ID

Major Changes 🔄

List highlighting major changes

  • Renames files from src/utilities/ga and src/utilities/ga.test.ts to src/utilities/analytics.ts and src/utilities/analytics.test.ts.
  • Renames some functions and comments around the app that mention "GA" to "Analytics" to avoid any future confusion.
  • Removes react-ga as a package.
  • Removes references to GA_ID and GTM_ID environment variables.
  • Removes unused custom events.
  • Restores firing of the sendCreateBucketEvent when an OBJ storage bucket is created -- we used to track this, but it seems to have gotten dropped when refactoring components.
  • Fixes a bug with the Disk Resize Flow custom event that was causing an event to incorrectly fire every time the error toast displayed, rather than every time the docs link was clicked on the error toast! (This was why we had an inflated count for this event.)

How to test 🧪

  1. How to setup test environment?
    • Have the REACT_APP_GA_ID set in your local .env. (Reach out if you need this.)
    • Have the REACT_APP_ADOBE_ANALYTICS_URL set in your local .env. (Reach out if you need this.)
    • yarn up
  2. How to verify changes?
    • Refer to src/utilities/analytics.ts for custom events to test.
    • Test actions that fire custom events and verify in the browser console that (1) no Google Analytics data is logged and (2) Adobe Analytics data is logged. See chore: [M3-6496] - Add Adobe Analytics custom event tracking #9004 for details instructions on setting up + expected output for Adobe logging.
  3. How to run Unit or E2E tests?
    yarn cy:run
  4. Review the corresponding internal PR (ask me for this) to get rid of our build env vars.

@mjac0bs mjac0bs self-assigned this Jun 15, 2023
@mjac0bs mjac0bs added the Analytics Relating to Analytics migration project or Adobe Analytics label Jun 15, 2023
@cypress
Copy link

cypress bot commented Jun 15, 2023

Passing run #4360 ↗︎

0 167 3 0 Flakiness 0

Details:

Merge branch 'develop' into new-M3-6497-ga-and-custom-events-clean-up
Project: Cloud Manager E2E Commit: 87bf1dfd38
Status: Passed Duration: 12:11 💡
Started: Jun 18, 2023 7:32 PM Ended: Jun 18, 2023 7:44 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Comment on lines -182 to -221

/*
* If in the future, we want to hook into every single
* async action for the purpose of sending the request data
* to Google Tag Manager, we can uncomment out the following
* .then() and .catch() on return Axios(config)
*/

// .then(response => {
// /*
// * This is sending an event to the Google Tag Manager
// * data layer. This is important because it lets us track
// * async actions as custom events
// */
// if ((window as any).dataLayer) {
// (window as any).dataLayer = (window as any).dataLayer || [];
// (window as any).dataLayer.push({
// 'event': 'asyncActionSuccess',
// 'url': response.config.url,
// 'method': response.config.method,
// });
// };
// return response;
// })
// .catch(e => {
// /*
// * This is sending an event to the Google Tag Manager
// * data layer. This is important because it lets us track
// * async actions as custom events
// */
// if ((window as any).dataLayer) {
// (window as any).dataLayer = (window as any).dataLayer || [];
// (window as any).dataLayer.push({
// 'event': 'asyncActionFailure',
// 'url': e.response.config.url,
// 'method': e.response.config.method,
// });
// };
// return Promise.reject(e);
// });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the removal of GA and GTM, we can remove this comment block - it's just taking up space.

Comment on lines -63 to -69
const fireGAEvent = (label: string) => {
sendEvent({
action: 'Click:link',
category: 'Linode Create API CLI Awareness Modal',
label,
});
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essentially used the sendApiAwarenessClick event in place of this function in order to keep events in src/utilities/analytics.tsx where we can.

action: `Click:link`,
label: 'Disk resize failed toast',
})
() =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the cause of a bug. 🙃 sendEvent was firing here every time the toast displayed, rather than just when the link on the toast was clicked.

Comment on lines +28 to +31
sendEvent({
category: 'Linode Action Menu',
action: 'Open Action Menu',
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was being called again with recent refactoring of the action menu, so I uncommented it here.

Comment on lines -85 to -130
// getAll.ts
export const sendFetchAllEvent = (
eventLabel: string,
eventValue: number
): void => {
sendEvent({
category: 'Search',
action: 'Data fetch all entities',
label: eventLabel,
value: eventValue,
});
};

// TagImportDrawer.tsx
export const sendImportDisplayGroupSubmitEvent = (
eventLabel: string,
eventValue: number
): void => {
sendEvent({
category: 'dashboard',
action: 'import display groups',
label: eventLabel,
value: eventValue,
});
};

// LinodeThemeWrapper.tsx
export const sendSpacingToggleEvent = (eventLabel: string): void => {
// AC 8/24/2020: disabling this event to reduce hits on GA as this seems to not be used
// sendEvent({
// category: 'Theme Choice',
// action: 'Spacing Toggle',
// label: eventLabel
// });
};

// LinodeThemeWrapper.tsx
export const sendThemeToggleEvent = (): void => {
// AC 9/24/2020: disabling this event to reduce hits on GA as this seems to not be used
// sendEvent({
// category: 'Theme Choice',
// action: 'Theme Toggle',
// label: eventLabel
// });
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of these events are currently in use, so I removed them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could also be a tech story or just not included at all in the changelog.

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with any of these options.

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with any of these options.

@bnussman-akamai bnussman-akamai added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Jun 15, 2023
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on this migration @mjac0bs

@bnussman-akamai bnussman-akamai added the Approved Multiple approvals and ready to merge! label Jun 15, 2023
@bnussman-akamai bnussman-akamai removed the Add'tl Approval Needed Waiting on another approval! label Jun 15, 2023
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

🎉

@@ -147,7 +136,9 @@ const ApiAwarenessModal = (props: Props) => {
<ExternalLink
text="personal access token"
link="/profile/tokens"
onClick={() => fireGAEvent('personal access token')}
onClick={() =>
sendApiAwarenessClickEvent('link', 'personal access token')
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, Is the first argument typed or can it be anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any string, to allow for an action like "Click:" where the input type could be a link, named tab, icon...

Copy link
Contributor

Choose a reason for hiding this comment

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

It’s unrelated to this PR but am wondering if we should eventually type this so we can chose from a limited set of events. The idea is that we can get dupes, events with different cases etc. And maybe in the future when we harvest it will be important to be able to filter events by types, or at least some events maybe be easier to group or just find. Just a thought!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's a good idea. Our current events are kind all over the place (especially in regard to actions), and I'm not sure how we'd best get those down to a limited set, but at the very least we could type-enforce for more consistency with new events. I'll document as a follow up task.

@mjac0bs mjac0bs merged commit f962d33 into linode:develop Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analytics Relating to Analytics migration project or Adobe Analytics Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants