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

change: [M3-8390] - Initialize Pendo on Cloud Manager #10982

Merged
merged 15 commits into from
Oct 7, 2024

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Sep 20, 2024

Description 📝

This PR adds Pendo to Cloud Manager by adding the Pendo agent script from the CDN and initializing Pendo with some custom configurations.

Changes 🔄

  • Installs js-sha256
  • Creates the usePendo hook
    • Incorporates Pendo's install script code
    • Loads the Pendo Agent from the CDN with loadScript
    • Initializes Pendo with some custom configuration
  • Invokes the hook in App.tsx with the rest of the GlobalListeners

Target release date 🗓️

10/14/24

How to test 🧪

Prerequisites

(How to setup test environment)

  • To your .env, add the Pendo API Key. This is not a secret key; it will be visible in the browser dev tools, but we also want to be able to stop Pendo from running using an environment variable, which will live elsewhere with our env var configurations. See the internal repo (PR 167) for that change and the API key.
# Pendo:
REACT_APP_PENDO_API_KEY='KEY-GOES-HERE'

Verification steps

(How to verify changes)

  • We can validate that Pendo is installed by opening the browser console and typing pendo.validateEnvironment(). This should output some data, including your hashed visitor and account id.
  • We also want to confirm that no sensitive data is sent to Pendo. To do this, filter by pendo in the Network tab of the browser dev tools.
    • Then try navigating around Cloud Manager and confirm the following by looking at the normalizedUrl of the requests:
      • No ids are being sent (e.g. when viewing a linode details page, linodes/123 should be truncated to linodes/)
      • No search queries are being sent
      • No username or other account data is being sent (e.g. when visiting the User Profile and User Permissions pages, the url doesn't include the username)
      • No token data is being sent

Example:
image

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@mjac0bs mjac0bs added Do Not Merge Analytics Relating to Analytics migration project or Adobe Analytics labels Sep 20, 2024
@mjac0bs mjac0bs self-assigned this Sep 20, 2024
@mjac0bs mjac0bs requested a review from a team as a code owner September 20, 2024 20:42
@mjac0bs mjac0bs requested review from bnussman-akamai and coliu-akamai and removed request for a team September 20, 2024 20:42
Comment on lines 33 to 60
// Adapted Pendo install script for readability:

// Set up Pendo namespace and queue
const pendo = (window['pendo'] = window['pendo'] || {});
pendo._q = pendo._q || [];

// Define the methods Pendo uses in a queue
const methodNames = [
'initialize',
'identify',
'updateOptions',
'pageLoad',
'track',
];

// Enqueue methods and their arguments on the Pendo object
methodNames.forEach((_, index) => {
(function (method) {
pendo[method] =
pendo[method] ||
function () {
pendo._q[method === methodNames[0] ? 'unshift' : 'push'](
// eslint-disable-next-line prefer-rest-params
[method].concat([].slice.call(arguments, 0))
);
};
})(methodNames[index]);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All this stuff is a slightly more readable version of the Pendo install script: https://support.pendo.io/hc/en-us/articles/21362607464987-Components-of-the-install-script.

Copy link
Contributor

@coliu-akamai coliu-akamai Oct 7, 2024

Choose a reason for hiding this comment

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

would we need to cite this in our code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need to as far as Pendo is concerned, but it's good context to have, since we set this up in a way that is consistent with our repo, deconstructing a bit to make use of our loadScript function. I added a link to the Pendo doc in a comment in the code so it's not just ephemerally in this PR - good call out.

loadScript(PENDO_URL, {
location: 'head',
}).then(() => {
window.pendo.initialize({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left the commented-out sections for reference.

packages/manager/package.json Show resolved Hide resolved

declare global {
interface Window {
pendo: any;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, Pendo docs recommend typing this as any.

Copy link

github-actions bot commented Sep 20, 2024

Coverage Report:
Base Coverage: 87.22%
Current Coverage: 86.94%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Script initialization looks good. Didn't 👁️ any issues

packages/manager/src/hooks/usePendo.ts Outdated Show resolved Hide resolved
@mjac0bs mjac0bs added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Oct 7, 2024
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

✅ confirmed pendo.validateEnvironment()
✅ confirmed no sensitive data is sent to Pendo

thanks Mariah! 🎉

@coliu-akamai coliu-akamai removed the Add'tl Approval Needed Waiting on another approval! label Oct 7, 2024
@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Oct 7, 2024
@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 7, 2024

Going to go ahead and merge this because CI was passing last week and today's updates shouldn't have impacted that; CI is just having known issues today.

@mjac0bs mjac0bs merged commit 7917700 into linode:develop Oct 7, 2024
17 of 20 checks passed
Copy link

cypress bot commented Oct 7, 2024

Cloud Manager E2E    Run #6633

Run Properties:  status check failed Failed #6633  •  git commit 79177001f8: change: [M3-8390] - Initialize Pendo on Cloud Manager (#10982)
Project Cloud Manager E2E
Run status status check failed Failed #6633
Run duration 29m 06s
Commit git commit 79177001f8: change: [M3-8390] - Initialize Pendo on Cloud Manager (#10982)
Committer Mariah Jacobs
View all properties for this run ↗︎

Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 423

Tests for review

Failed  linodes/clone-linode.spec.ts • 1 failed test

View Output Video

Test Artifacts
clone linode > can clone a Linode from Linode details page Screenshots Video
Failed  placementGroups/delete-placement-groups.spec.ts • 1 failed test

View Output Video

Test Artifacts
Placement Group deletion > can delete with Linodes assigned when unexpected error show up and retry Screenshots Video
Flakiness  linodes/rebuild-linode.spec.ts • 1 flaky test

View Output Video

Test Artifacts
rebuild linode > cannot rebuild a provisioning linode Screenshots Video
Flakiness  images/machine-image-upload.spec.ts • 1 flaky test

View Output Video

Test Artifacts
machine image > uploads machine image, mock upload canceled failed event Screenshots Video
Flakiness  parentChild/account-switching.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Parent/Child account switching > From Parent to Child > can switch from Parent account user to Proxy account user from Billing page Screenshots Video

hasyed-akamai pushed a commit to hasyed-akamai/manager that referenced this pull request Oct 9, 2024
* Save work

* Adapt install script and get usePendo hook working

* Clean up logging

* Add sha256 hashing for ids

* Clean up for readability

* Sanitize the URLs

* Add env var to .env.example

* Added changeset: Add Pendo to Cloud Manager

* Make IDs unique in multiple environments

* Address feedback: add auth matching to regex

* Address feedback: reference link in code comment for context
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.

3 participants