From 95dccda6bef3d976ade9f49a2f2c3a529054084b Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 10 Jan 2025 15:38:13 -0500 Subject: [PATCH 1/5] Implement EventTracker abstraction and Amplitude implementation --- src/index.tsx | 2 + .../components/UserDropdown/UserDropdown.tsx | 12 ++- src/services/events/amplitude/amplitude.ts | 80 +++++++++++++++++++ src/services/events/events.ts | 54 +++++++++++++ src/services/user/useUser.ts | 21 ++++- .../ListRepo/InactiveRepo/InactiveRepo.tsx | 10 +++ src/shared/utils/provider.ts | 72 ++++++++++------- src/ui/ContextSwitcher/ContextSwitcher.tsx | 9 ++- 8 files changed, 225 insertions(+), 35 deletions(-) create mode 100644 src/services/events/amplitude/amplitude.ts create mode 100644 src/services/events/events.ts diff --git a/src/index.tsx b/src/index.tsx index 904a79f97f..6e68042f99 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -12,6 +12,7 @@ import { Router } from 'react-router-dom' import { CompatRouter } from 'react-router-dom-v5-compat' import ErrorBoundary from 'layouts/shared/ErrorBoundary' +import { initEventTracker } from 'services/events/events' import { withFeatureFlagProvider } from 'shared/featureFlags' import App from './App' @@ -38,6 +39,7 @@ const history = createBrowserHistory() const TOO_MANY_REQUESTS_ERROR_CODE = 429 +initEventTracker() setupSentry({ history }) const queryClient = new QueryClient({ diff --git a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx index b40e669074..c10c584081 100644 --- a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx +++ b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx @@ -2,6 +2,7 @@ import { useHistory, useParams } from 'react-router-dom' import config from 'config' +import { eventTracker } from 'services/events/events' import { useUser } from 'services/user' import { Provider } from 'shared/api/helpers' import { providerToName } from 'shared/utils/provider' @@ -11,6 +12,8 @@ import { Dropdown } from 'ui/Dropdown/Dropdown' interface URLParams { provider: Provider + owner: string + repo?: string } type DropdownItem = { @@ -32,8 +35,8 @@ function UserDropdown() { }, }) - const { provider } = useParams() - const isGh = providerToName(provider) === 'GitHub' + const { provider, owner, repo } = useParams() + const isGh = providerToName(provider) === 'Github' const history = useHistory() const items = @@ -42,6 +45,11 @@ function UserDropdown() { { to: { pageName: 'codecovAppInstallation' }, children: 'Install Codecov app', + onClick: () => + eventTracker(provider, owner, repo).track('Button Clicked', { + buttonType: 'Install Github App', + buttonLocation: 'UserDropdown', + }), } as DropdownItem, ] : [] diff --git a/src/services/events/amplitude/amplitude.ts b/src/services/events/amplitude/amplitude.ts new file mode 100644 index 0000000000..e5d4a18184 --- /dev/null +++ b/src/services/events/amplitude/amplitude.ts @@ -0,0 +1,80 @@ +import * as amplitude from '@amplitude/analytics-browser' + +import { Provider } from 'shared/api/helpers' +import { + InternalProvider, + providerToInternalProvider, +} from 'shared/utils/provider' + +import { EventTracker } from '../events' + +const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY + +export function initAmplitude() { + if (!AMPLITUDE_API_KEY) { + return + } + amplitude.init(AMPLITUDE_API_KEY, { + // Disable all autocapture - may change this in the future + autocapture: false, + minIdLength: 1, // Necessary to accommodate our owner ids + }) +} + +export class AmplitudeEventTracker implements EventTracker { + #provider?: InternalProvider + #owner?: string + #providerOwner?: string + #repo?: string + + constructor(provider?: Provider, owner?: string, repo?: string) { + if (!AMPLITUDE_API_KEY) { + throw new Error( + 'AMPLITUDE_API_KEY is not defined. Ensure the environment variable is defined before attempting to initialize AmplitudeEventTracker.' + ) + } + this.#provider = provider ? providerToInternalProvider(provider) : undefined + this.#owner = owner + this.#providerOwner = + this.#provider && this.#owner + ? formatProviderOwner(this.#provider, this.#owner) + : undefined + this.#repo = repo + } + + identify(userOwnerId: number | string, username: string) { + amplitude.setUserId(userOwnerId.toString()) + const identifyEvent = new amplitude.Identify() + if (this.#provider) { + identifyEvent.set('provider', this.#provider) + } + identifyEvent.set('username', username) + amplitude.identify(identifyEvent) + } + + track(eventType: string, eventProperties: any) { + amplitude.track({ + // eslint-disable-next-line camelcase + event_type: eventType, + // eslint-disable-next-line camelcase + event_properties: { + owner: this.#providerOwner, + repo: this.#repo, + ...eventProperties, + }, + // This attaches the event to the owner's user group as well + groups: this.#providerOwner + ? { + owner: this.#providerOwner, + } + : undefined, + }) + } +} + +function formatProviderOwner(provider: InternalProvider, ownerName: string) { + // Helper function to format the owner group name. + // The reason for this is owner names are not unique on their own, but + // provider/owner names are. + return `${provider}/${ownerName}` +} diff --git a/src/services/events/events.ts b/src/services/events/events.ts new file mode 100644 index 0000000000..76763680d1 --- /dev/null +++ b/src/services/events/events.ts @@ -0,0 +1,54 @@ +import { Provider } from 'shared/api/helpers' + +import { AmplitudeEventTracker, initAmplitude } from './amplitude/amplitude' + +const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY + +export interface EventTracker { + // Identifies the user this session belongs to. + identify(userOwnerId: number | string, username: string): void + + // Add new events as a new overloaded method signature here. + // Please keep the `eventType`s very generic as we have a limited number of + // them. Instead, add more detail in `eventProperties` where possible. + // Adding event types this way provides type safety for event properties. + // E.g., every 'Button Clicked' event must have the buttonType property. + track( + eventType: 'Button Clicked', + eventProperties: { + buttonType: 'Install Github App' | 'Configure Repo' + buttonLocation?: string + } + ): void + track( + eventType: 'Page Viewed', + eventProperties: { + pageName: 'OwnerPage' + } + ): void +} + +class StubbedEventTracker implements EventTracker { + identify(_userOwnerId: string | number, _username: string): void {} + track(_eventType: string, _eventProperties: any): void {} +} + +export function initEventTracker(): void { + // Calls any init functions for EventTracker implementations + initAmplitude() +} + +// Returns an EventTracker. Provide any of provider, owner, repo that are +// relevant to the event you're going to track. They will be attached to +// any events you send with the returned EventTracker instance. +export function eventTracker( + provider?: Provider, + owner?: string, + repo?: string +): EventTracker { + if (!AMPLITUDE_API_KEY) { + // If the API key is not defined, we'll just return a stubbed EventTracker. + return new StubbedEventTracker() + } + return new AmplitudeEventTracker(provider, owner, repo) +} diff --git a/src/services/user/useUser.ts b/src/services/user/useUser.ts index cfaf131aa1..8529daf13e 100644 --- a/src/services/user/useUser.ts +++ b/src/services/user/useUser.ts @@ -2,8 +2,9 @@ import { useQuery } from '@tanstack/react-query' import { useParams } from 'react-router-dom' import { z } from 'zod' +import { eventTracker } from 'services/events/events' import Api from 'shared/api' -import { NetworkErrorObject } from 'shared/api/helpers' +import { NetworkErrorObject, Provider } from 'shared/api/helpers' export const TypeProjectsSchema = z.array( z.union([ @@ -27,6 +28,8 @@ export const GoalsSchema = z.array( const MeSchema = z.object({ owner: z.object({ defaultOrgUsername: z.string().nullable(), + ownerid: z.number().nullable(), + username: z.string().nullable(), }), email: z.string().nullable(), privateAccess: z.boolean().nullable(), @@ -80,6 +83,8 @@ const currentUserFragment = ` fragment CurrentUserFragment on Me { owner { defaultOrgUsername + ownerid + username } email privateAccess @@ -121,7 +126,8 @@ fragment CurrentUserFragment on Me { ` interface URLParams { - provider: string + provider: Provider + owner: string } interface UseUserArgs { @@ -134,7 +140,6 @@ interface UseUserArgs { export function useUser({ options }: UseUserArgs = {}) { const { provider } = useParams() - const query = ` query CurrentUser { me { @@ -158,6 +163,16 @@ export function useUser({ options }: UseUserArgs = {}) { } satisfies NetworkErrorObject) } + if ( + parsedRes.data.me?.owner.ownerid && + parsedRes.data.me?.owner.username + ) { + eventTracker(provider).identify( + parsedRes.data.me.owner.ownerid, + parsedRes.data.me.owner.username + ) + } + return parsedRes.data.me }), enabled: provider !== undefined, diff --git a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx index 2ab5351876..e7686a340d 100644 --- a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx +++ b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx @@ -1,3 +1,7 @@ +import { useParams } from 'react-router' + +import { eventTracker } from 'services/events/events' +import { Provider } from 'shared/api/helpers' import A from 'ui/A' function InactiveRepo({ @@ -11,6 +15,7 @@ function InactiveRepo({ repoName?: string isCurrentUserPartOfOrg?: boolean }) { + const { provider } = useParams<{ provider: Provider }>() if (isActive) return <>Deactivated if (!isCurrentUserPartOfOrg) return <>Inactive @@ -26,6 +31,11 @@ function InactiveRepo({ repo: repoName, }, }} + onClick={() => + eventTracker(provider, owner, repoName).track('Button Clicked', { + buttonType: 'Configure Repo', + }) + } > Configure diff --git a/src/shared/utils/provider.ts b/src/shared/utils/provider.ts index 2110a9b611..1bcf645a68 100644 --- a/src/shared/utils/provider.ts +++ b/src/shared/utils/provider.ts @@ -4,37 +4,51 @@ import config from 'config' import { Provider } from 'shared/api/helpers' export function providerToName(provider: Provider) { - return { - gh: 'GitHub', - bb: 'Bitbucket', - gl: 'GitLab', - ghe: 'GitHub Enterprise', - gle: 'GitLab Enterprise', - bbs: 'Bitbucket Server', - github: 'GitHub', - bitbucket: 'Bitbucket', - gitlab: 'GitLab', - github_enterprise: 'GitHub Enterprise', - gitlab_enterprise: 'GitLab Enterprise', - bitbucket_server: 'Bitbucket Server', - }[provider.toLowerCase()] + return ( + { + gh: 'Github', + bb: 'BitBucket', + gl: 'Gitlab', + ghe: 'Github Enterprise', + gle: 'Gitlab Enterprise', + bbs: 'BitBucket Server', + github: 'Github', + bitbucket: 'BitBucket', + gitlab: 'Gitlab', + github_enterprise: 'Github Enterprise', + gitlab_enterprise: 'Gitlab Enterprise', + bitbucket_server: 'BitBucket Server', + } as const + )[provider] } -export function providerToInternalProvider(provider: Provider) { - return { - gh: 'github', - bb: 'bitbucket', - gl: 'gitlab', - ghe: 'github_enterprise', - gle: 'gitlab_enterprise', - bbs: 'bitbucket_server', - github: 'github', - bitbucket: 'bitbucket', - gitlab: 'gitlab', - github_enterprise: 'github_enterprise', - gitlab_enterprise: 'gitlab_enterprise', - bitbucket_server: 'bitbucket_server', - }[provider.toLowerCase()] +export type InternalProvider = + | 'github' + | 'bitbucket' + | 'gitlab' + | 'github_enterprise' + | 'gitlab_enterprise' + | 'bitbucket_server' + +export function providerToInternalProvider( + provider: Provider +): InternalProvider { + return ( + { + gh: 'github', + bb: 'bitbucket', + gl: 'gitlab', + ghe: 'github_enterprise', + gle: 'gitlab_enterprise', + bbs: 'bitbucket_server', + github: 'github', + bitbucket: 'bitbucket', + gitlab: 'gitlab', + github_enterprise: 'github_enterprise', + gitlab_enterprise: 'gitlab_enterprise', + bitbucket_server: 'bitbucket_server', + } as const + )[provider] } export function getProviderCommitURL({ diff --git a/src/ui/ContextSwitcher/ContextSwitcher.tsx b/src/ui/ContextSwitcher/ContextSwitcher.tsx index e506be53fb..49717a74a7 100644 --- a/src/ui/ContextSwitcher/ContextSwitcher.tsx +++ b/src/ui/ContextSwitcher/ContextSwitcher.tsx @@ -7,6 +7,7 @@ import useClickAway from 'react-use/lib/useClickAway' import config, { DEFAULT_GH_APP } from 'config' import { useUpdateDefaultOrganization } from 'services/defaultOrganization' +import { eventTracker } from 'services/events/events' import { Provider } from 'shared/api/helpers' import { providerToName } from 'shared/utils/provider' import A from 'ui/A' @@ -170,7 +171,7 @@ function ContextSwitcher({ const intersectionRef = useLoadMore({ onLoadMore }) const defaultOrgUsername = currentUser?.defaultOrgUsername - const isGh = providerToName(provider) === 'GitHub' + const isGh = providerToName(provider) === 'Github' const isSelfHosted = config.IS_SELF_HOSTED const isCustomGitHubApp = config.GH_APP !== DEFAULT_GH_APP @@ -217,6 +218,12 @@ function ContextSwitcher({
  • + eventTracker(provider, owner).track('Button Clicked', { + buttonType: 'Install Github App', + buttonLocation: 'ContextSwitcher', + }) + } isExternal hook="context-switcher-gh-install-link" > From 9f549ef1031a9986cf22484cc6d9bec565f9716c Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Mon, 13 Jan 2025 10:36:39 -0500 Subject: [PATCH 2/5] Ajay changes pt1 --- src/services/events/events.ts | 8 ++------ src/services/events/stub.ts | 6 ++++++ src/shared/utils/provider.ts | 24 ++++++++++++------------ 3 files changed, 20 insertions(+), 18 deletions(-) create mode 100644 src/services/events/stub.ts diff --git a/src/services/events/events.ts b/src/services/events/events.ts index 76763680d1..6fc1839b77 100644 --- a/src/services/events/events.ts +++ b/src/services/events/events.ts @@ -1,12 +1,13 @@ import { Provider } from 'shared/api/helpers' import { AmplitudeEventTracker, initAmplitude } from './amplitude/amplitude' +import { StubbedEventTracker } from './stub' const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY export interface EventTracker { // Identifies the user this session belongs to. - identify(userOwnerId: number | string, username: string): void + identify(userOwnerId: number, username: string): void // Add new events as a new overloaded method signature here. // Please keep the `eventType`s very generic as we have a limited number of @@ -28,11 +29,6 @@ export interface EventTracker { ): void } -class StubbedEventTracker implements EventTracker { - identify(_userOwnerId: string | number, _username: string): void {} - track(_eventType: string, _eventProperties: any): void {} -} - export function initEventTracker(): void { // Calls any init functions for EventTracker implementations initAmplitude() diff --git a/src/services/events/stub.ts b/src/services/events/stub.ts new file mode 100644 index 0000000000..d5216f1743 --- /dev/null +++ b/src/services/events/stub.ts @@ -0,0 +1,6 @@ +import { EventTracker } from './events' + +export class StubbedEventTracker implements EventTracker { + identify(_userOwnerId: string | number, _username: string): void {} + track(_eventType: string, _eventProperties: any): void {} +} diff --git a/src/shared/utils/provider.ts b/src/shared/utils/provider.ts index 1bcf645a68..3ff6672ee4 100644 --- a/src/shared/utils/provider.ts +++ b/src/shared/utils/provider.ts @@ -6,18 +6,18 @@ import { Provider } from 'shared/api/helpers' export function providerToName(provider: Provider) { return ( { - gh: 'Github', - bb: 'BitBucket', - gl: 'Gitlab', - ghe: 'Github Enterprise', - gle: 'Gitlab Enterprise', - bbs: 'BitBucket Server', - github: 'Github', - bitbucket: 'BitBucket', - gitlab: 'Gitlab', - github_enterprise: 'Github Enterprise', - gitlab_enterprise: 'Gitlab Enterprise', - bitbucket_server: 'BitBucket Server', + gh: 'GitHub', + bb: 'Bitbucket', + gl: 'GitLab', + ghe: 'GitHub Enterprise', + gle: 'GitLab Enterprise', + bbs: 'Bitbucket Server', + github: 'GitHub', + bitbucket: 'Bitbucket', + gitlab: 'GitLab', + github_enterprise: 'GitHub Enterprise', + gitlab_enterprise: 'GitLab Enterprise', + bitbucket_server: 'Bitbucket Server', } as const )[provider] } From 8f189a6190af8ceaedc515f71437ea859e67e09d Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Mon, 13 Jan 2025 11:30:04 -0500 Subject: [PATCH 3/5] Convert EventTracker to abstract class and object payloads --- .../components/UserDropdown/UserDropdown.tsx | 11 ++++-- .../GithubConfigBanner/GithubConfigBanner.jsx | 12 +++++- src/services/events/amplitude/amplitude.ts | 15 ++++++-- src/services/events/events.ts | 38 +++++++++---------- src/services/events/stub.ts | 11 +++++- src/services/events/types.ts | 21 ++++++++++ src/services/user/useUser.ts | 8 ++-- .../ListRepo/InactiveRepo/InactiveRepo.tsx | 7 +++- src/ui/ContextSwitcher/ContextSwitcher.tsx | 11 ++++-- 9 files changed, 93 insertions(+), 41 deletions(-) create mode 100644 src/services/events/types.ts diff --git a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx index c10c584081..cbefe054e4 100644 --- a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx +++ b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx @@ -36,7 +36,7 @@ function UserDropdown() { }) const { provider, owner, repo } = useParams() - const isGh = providerToName(provider) === 'Github' + const isGh = providerToName(provider) === 'GitHub' const history = useHistory() const items = @@ -46,9 +46,12 @@ function UserDropdown() { to: { pageName: 'codecovAppInstallation' }, children: 'Install Codecov app', onClick: () => - eventTracker(provider, owner, repo).track('Button Clicked', { - buttonType: 'Install Github App', - buttonLocation: 'UserDropdown', + eventTracker(provider, owner, repo).track({ + type: 'Button Clicked', + properties: { + buttonType: 'Install Github App', + buttonLocation: 'UserDropdown', + }, }), } as DropdownItem, ] diff --git a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx index 60b17e8251..45d3f238ee 100644 --- a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx +++ b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx @@ -1,5 +1,6 @@ import { useParams } from 'react-router-dom' +import { eventTracker } from 'services/events/events' import { providerToName } from 'shared/utils/provider' import A from 'ui/A' import Banner from 'ui/Banner' @@ -7,7 +8,7 @@ import BannerContent from 'ui/Banner/BannerContent' import BannerHeading from 'ui/Banner/BannerHeading' const GithubConfigBanner = () => { - const { provider } = useParams() + const { provider, owner } = useParams() const isGh = providerToName(provider) === 'GitHub' if (!isGh) return null @@ -21,6 +22,15 @@ const GithubConfigBanner = () => { + eventTracker(provider, owner).track({ + type: 'Button Clicked', + properties: { + buttonType: 'Install Github App', + buttonLocation: 'GithubConfigBanner', + }, + }) + } > Codecov's GitHub app diff --git a/src/services/events/amplitude/amplitude.ts b/src/services/events/amplitude/amplitude.ts index e5d4a18184..3440846d44 100644 --- a/src/services/events/amplitude/amplitude.ts +++ b/src/services/events/amplitude/amplitude.ts @@ -7,6 +7,7 @@ import { } from 'shared/utils/provider' import { EventTracker } from '../events' +import { Event } from '../types' const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY @@ -42,7 +43,13 @@ export class AmplitudeEventTracker implements EventTracker { this.#repo = repo } - identify(userOwnerId: number | string, username: string) { + identify({ + userOwnerId, + username, + }: { + userOwnerId: number + username: string + }) { amplitude.setUserId(userOwnerId.toString()) const identifyEvent = new amplitude.Identify() if (this.#provider) { @@ -52,15 +59,15 @@ export class AmplitudeEventTracker implements EventTracker { amplitude.identify(identifyEvent) } - track(eventType: string, eventProperties: any) { + track(event: Event) { amplitude.track({ // eslint-disable-next-line camelcase - event_type: eventType, + event_type: event.type, // eslint-disable-next-line camelcase event_properties: { owner: this.#providerOwner, repo: this.#repo, - ...eventProperties, + ...event.properties, }, // This attaches the event to the owner's user group as well groups: this.#providerOwner diff --git a/src/services/events/events.ts b/src/services/events/events.ts index 6fc1839b77..bc2253473b 100644 --- a/src/services/events/events.ts +++ b/src/services/events/events.ts @@ -2,31 +2,29 @@ import { Provider } from 'shared/api/helpers' import { AmplitudeEventTracker, initAmplitude } from './amplitude/amplitude' import { StubbedEventTracker } from './stub' +import { Event } from './types' const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY -export interface EventTracker { +export abstract class EventTracker { // Identifies the user this session belongs to. - identify(userOwnerId: number, username: string): void + identify({ + userOwnerId: _userOwnerId, + username: _username, + }: { + userOwnerId: number + username: string + }): void { + throw new Error( + 'EventTracker is abstract. Method identify must be implemented.' + ) + } - // Add new events as a new overloaded method signature here. - // Please keep the `eventType`s very generic as we have a limited number of - // them. Instead, add more detail in `eventProperties` where possible. - // Adding event types this way provides type safety for event properties. - // E.g., every 'Button Clicked' event must have the buttonType property. - track( - eventType: 'Button Clicked', - eventProperties: { - buttonType: 'Install Github App' | 'Configure Repo' - buttonLocation?: string - } - ): void - track( - eventType: 'Page Viewed', - eventProperties: { - pageName: 'OwnerPage' - } - ): void + track(_event: Event): void { + throw new Error( + 'EventTracker is abstract. Method track must be implemented.' + ) + } } export function initEventTracker(): void { diff --git a/src/services/events/stub.ts b/src/services/events/stub.ts index d5216f1743..4735d4a17c 100644 --- a/src/services/events/stub.ts +++ b/src/services/events/stub.ts @@ -1,6 +1,13 @@ import { EventTracker } from './events' +import { Event } from './types' export class StubbedEventTracker implements EventTracker { - identify(_userOwnerId: string | number, _username: string): void {} - track(_eventType: string, _eventProperties: any): void {} + identify({ + userOwnerId: _userOwnerId, + username: _username, + }: { + userOwnerId: number + username: string + }): void {} + track(_event: Event): void {} } diff --git a/src/services/events/types.ts b/src/services/events/types.ts new file mode 100644 index 0000000000..33aef3cdd5 --- /dev/null +++ b/src/services/events/types.ts @@ -0,0 +1,21 @@ +// Add new events to the union type here. +// Please keep the values of `type` very generic as we have a limited number of +// them. Instead, add more detail in `properties` where possible. +// Adding event types this way provides type safety for names and event +// properties. +// E.g., every 'Button Clicked' event must have the buttonType property. + +export type Event = + | { + type: 'Button Clicked' + properties: { + buttonType: 'Install Github App' | 'Configure Repo' + buttonLocation?: string + } + } + | { + type: 'Page Viewed' + properties: { + pageName: 'OwnerPage' + } + } diff --git a/src/services/user/useUser.ts b/src/services/user/useUser.ts index 8529daf13e..733bb0a277 100644 --- a/src/services/user/useUser.ts +++ b/src/services/user/useUser.ts @@ -167,10 +167,10 @@ export function useUser({ options }: UseUserArgs = {}) { parsedRes.data.me?.owner.ownerid && parsedRes.data.me?.owner.username ) { - eventTracker(provider).identify( - parsedRes.data.me.owner.ownerid, - parsedRes.data.me.owner.username - ) + eventTracker(provider).identify({ + userOwnerId: parsedRes.data.me.owner.ownerid, + username: parsedRes.data.me.owner.username, + }) } return parsedRes.data.me diff --git a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx index e7686a340d..577774b759 100644 --- a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx +++ b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx @@ -32,8 +32,11 @@ function InactiveRepo({ }, }} onClick={() => - eventTracker(provider, owner, repoName).track('Button Clicked', { - buttonType: 'Configure Repo', + eventTracker(provider, owner, repoName).track({ + type: 'Button Clicked', + properties: { + buttonType: 'Configure Repo', + }, }) } > diff --git a/src/ui/ContextSwitcher/ContextSwitcher.tsx b/src/ui/ContextSwitcher/ContextSwitcher.tsx index 49717a74a7..283382ce64 100644 --- a/src/ui/ContextSwitcher/ContextSwitcher.tsx +++ b/src/ui/ContextSwitcher/ContextSwitcher.tsx @@ -171,7 +171,7 @@ function ContextSwitcher({ const intersectionRef = useLoadMore({ onLoadMore }) const defaultOrgUsername = currentUser?.defaultOrgUsername - const isGh = providerToName(provider) === 'Github' + const isGh = providerToName(provider) === 'GitHub' const isSelfHosted = config.IS_SELF_HOSTED const isCustomGitHubApp = config.GH_APP !== DEFAULT_GH_APP @@ -219,9 +219,12 @@ function ContextSwitcher({ - eventTracker(provider, owner).track('Button Clicked', { - buttonType: 'Install Github App', - buttonLocation: 'ContextSwitcher', + eventTracker(provider, owner).track({ + type: 'Button Clicked', + properties: { + buttonType: 'Install Github App', + buttonLocation: 'ContextSwitcher', + }, }) } isExternal From 6e309e5104369d6af6b436a0d8be42b0408d53aa Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Mon, 13 Jan 2025 16:34:16 -0500 Subject: [PATCH 4/5] Iterate on EventTracker --- src/layouts/BaseLayout/BaseLayout.tsx | 2 + .../components/UserDropdown/UserDropdown.tsx | 8 +- .../GithubConfigBanner/GithubConfigBanner.jsx | 4 +- src/services/events/amplitude/amplitude.ts | 68 +++++--------- src/services/events/events.ts | 92 +++++++++++-------- src/services/events/stub.ts | 12 +-- src/services/events/types.ts | 65 ++++++++++++- src/services/repo/useRepo.tsx | 3 + src/services/user/useOwner.ts | 2 +- src/services/user/useUser.ts | 4 +- .../ListRepo/InactiveRepo/InactiveRepo.tsx | 7 +- src/ui/ContextSwitcher/ContextSwitcher.tsx | 6 +- 12 files changed, 157 insertions(+), 116 deletions(-) diff --git a/src/layouts/BaseLayout/BaseLayout.tsx b/src/layouts/BaseLayout/BaseLayout.tsx index 6aed3fb9fe..860a944942 100644 --- a/src/layouts/BaseLayout/BaseLayout.tsx +++ b/src/layouts/BaseLayout/BaseLayout.tsx @@ -10,6 +10,7 @@ import NetworkErrorBoundary from 'layouts/shared/NetworkErrorBoundary' import SilentNetworkErrorWrapper from 'layouts/shared/SilentNetworkErrorWrapper' import ToastNotifications from 'layouts/ToastNotifications' import { RepoBreadcrumbProvider } from 'pages/RepoPage/context' +import { useEventContext } from 'services/events/events' import { useImpersonate } from 'services/impersonate' import { useTracking } from 'services/tracking' import GlobalBanners from 'shared/GlobalBanners' @@ -77,6 +78,7 @@ interface URLParams { function BaseLayout({ children }: React.PropsWithChildren) { const { provider, owner, repo } = useParams() useTracking() + useEventContext() const { isImpersonating } = useImpersonate() const { isFullExperience, diff --git a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx index cbefe054e4..e9e5a4aa60 100644 --- a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx +++ b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx @@ -35,7 +35,7 @@ function UserDropdown() { }, }) - const { provider, owner, repo } = useParams() + const { provider } = useParams() const isGh = providerToName(provider) === 'GitHub' const history = useHistory() @@ -46,11 +46,11 @@ function UserDropdown() { to: { pageName: 'codecovAppInstallation' }, children: 'Install Codecov app', onClick: () => - eventTracker(provider, owner, repo).track({ + eventTracker().track({ type: 'Button Clicked', properties: { - buttonType: 'Install Github App', - buttonLocation: 'UserDropdown', + buttonType: 'Install GitHub App', + buttonLocation: 'User dropdown', }, }), } as DropdownItem, diff --git a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx index 45d3f238ee..6aba1c0ef4 100644 --- a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx +++ b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx @@ -26,8 +26,8 @@ const GithubConfigBanner = () => { eventTracker(provider, owner).track({ type: 'Button Clicked', properties: { - buttonType: 'Install Github App', - buttonLocation: 'GithubConfigBanner', + buttonType: 'Install GitHub App', + buttonLocation: 'Configure GitHub app banner', }, }) } diff --git a/src/services/events/amplitude/amplitude.ts b/src/services/events/amplitude/amplitude.ts index 3440846d44..35f00553d6 100644 --- a/src/services/events/amplitude/amplitude.ts +++ b/src/services/events/amplitude/amplitude.ts @@ -1,19 +1,16 @@ import * as amplitude from '@amplitude/analytics-browser' -import { Provider } from 'shared/api/helpers' -import { - InternalProvider, - providerToInternalProvider, -} from 'shared/utils/provider' +import { providerToInternalProvider } from 'shared/utils/provider' -import { EventTracker } from '../events' -import { Event } from '../types' +import { Event, EventContext, EventTracker, Identity } from '../types' const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY export function initAmplitude() { if (!AMPLITUDE_API_KEY) { - return + throw new Error( + 'AMPLITUDE_API_KEY is not defined. Amplitude events will not be tracked.' + ) } amplitude.init(AMPLITUDE_API_KEY, { // Disable all autocapture - may change this in the future @@ -23,40 +20,21 @@ export function initAmplitude() { } export class AmplitudeEventTracker implements EventTracker { - #provider?: InternalProvider - #owner?: string - #providerOwner?: string - #repo?: string + context: EventContext = {} + identity?: Identity - constructor(provider?: Provider, owner?: string, repo?: string) { - if (!AMPLITUDE_API_KEY) { - throw new Error( - 'AMPLITUDE_API_KEY is not defined. Ensure the environment variable is defined before attempting to initialize AmplitudeEventTracker.' - ) + identify(identity: Identity) { + if (JSON.stringify(this.identity) === JSON.stringify(identity)) { + // Don't identify this user again this session. + return } - this.#provider = provider ? providerToInternalProvider(provider) : undefined - this.#owner = owner - this.#providerOwner = - this.#provider && this.#owner - ? formatProviderOwner(this.#provider, this.#owner) - : undefined - this.#repo = repo - } - identify({ - userOwnerId, - username, - }: { - userOwnerId: number - username: string - }) { - amplitude.setUserId(userOwnerId.toString()) + amplitude.setUserId(identity.userOwnerId.toString()) const identifyEvent = new amplitude.Identify() - if (this.#provider) { - identifyEvent.set('provider', this.#provider) - } - identifyEvent.set('username', username) + identifyEvent.set('provider', providerToInternalProvider(identity.provider)) amplitude.identify(identifyEvent) + + this.identity = identity } track(event: Event) { @@ -65,23 +43,19 @@ export class AmplitudeEventTracker implements EventTracker { event_type: event.type, // eslint-disable-next-line camelcase event_properties: { - owner: this.#providerOwner, - repo: this.#repo, ...event.properties, + ...this.context, }, // This attaches the event to the owner's user group as well - groups: this.#providerOwner + groups: this.context.owner?.id ? { - owner: this.#providerOwner, + owner: this.context.owner.id, } : undefined, }) } -} -function formatProviderOwner(provider: InternalProvider, ownerName: string) { - // Helper function to format the owner group name. - // The reason for this is owner names are not unique on their own, but - // provider/owner names are. - return `${provider}/${ownerName}` + setContext(context: EventContext) { + this.context = context + } } diff --git a/src/services/events/events.ts b/src/services/events/events.ts index bc2253473b..473792cab6 100644 --- a/src/services/events/events.ts +++ b/src/services/events/events.ts @@ -1,48 +1,64 @@ -import { Provider } from 'shared/api/helpers' +import { captureException } from '@sentry/react' +import { useEffect } from 'react' +import { useParams } from 'react-router' + +import { useRepo } from 'services/repo' +import { useOwner } from 'services/user' import { AmplitudeEventTracker, initAmplitude } from './amplitude/amplitude' import { StubbedEventTracker } from './stub' -import { Event } from './types' - -const AMPLITUDE_API_KEY = process.env.REACT_APP_AMPLITUDE_API_KEY - -export abstract class EventTracker { - // Identifies the user this session belongs to. - identify({ - userOwnerId: _userOwnerId, - username: _username, - }: { - userOwnerId: number - username: string - }): void { - throw new Error( - 'EventTracker is abstract. Method identify must be implemented.' - ) - } +import { EventTracker } from './types' - track(_event: Event): void { - throw new Error( - 'EventTracker is abstract. Method track must be implemented.' - ) +// EventTracker singleton +let EVENT_TRACKER: EventTracker = new StubbedEventTracker() + +export function initEventTracker(): void { + // Sets the global EventTracker singleton and calls necessary init functions + try { + initAmplitude() + EVENT_TRACKER = new AmplitudeEventTracker() + } catch (e) { + if (process.env.REACT_APP_ENV === 'production') { + // If in production, we need to know this has occured. + captureException(e) + } } } -export function initEventTracker(): void { - // Calls any init functions for EventTracker implementations - initAmplitude() +// Returns the global EventTracker instance. +export function eventTracker(): EventTracker { + return EVENT_TRACKER } -// Returns an EventTracker. Provide any of provider, owner, repo that are -// relevant to the event you're going to track. They will be attached to -// any events you send with the returned EventTracker instance. -export function eventTracker( - provider?: Provider, - owner?: string, - repo?: string -): EventTracker { - if (!AMPLITUDE_API_KEY) { - // If the API key is not defined, we'll just return a stubbed EventTracker. - return new StubbedEventTracker() - } - return new AmplitudeEventTracker(provider, owner, repo) +// Hook to keep the global EventTracker's context up-to-date. +export function useEventContext() { + const { provider, owner, repo } = useParams<{ + provider?: string + owner?: string + repo?: string + }>() + + const { data: ownerData } = useOwner({ username: owner }) + const { data: repoData } = useRepo({ + provider: provider || '', + owner: owner || '', + repo: repo || '', + opts: { enabled: !!(provider && owner && repo) }, + }) + + useEffect(() => { + EVENT_TRACKER.setContext({ + owner: ownerData?.ownerid + ? { + id: ownerData?.ownerid, + } + : undefined, + repo: repoData?.repository + ? { + id: repoData?.repository?.repoid, + isPrivate: repoData?.repository.private || undefined, + } + : undefined, + }) + }, [ownerData, repoData]) } diff --git a/src/services/events/stub.ts b/src/services/events/stub.ts index 4735d4a17c..53876068c5 100644 --- a/src/services/events/stub.ts +++ b/src/services/events/stub.ts @@ -1,13 +1,7 @@ -import { EventTracker } from './events' -import { Event } from './types' +import { Event, EventContext, EventTracker, Identity } from './types' export class StubbedEventTracker implements EventTracker { - identify({ - userOwnerId: _userOwnerId, - username: _username, - }: { - userOwnerId: number - username: string - }): void {} + identify(_identity: Identity): void {} track(_event: Event): void {} + setContext(_context: EventContext): void {} } diff --git a/src/services/events/types.ts b/src/services/events/types.ts index 33aef3cdd5..2cf9baa571 100644 --- a/src/services/events/types.ts +++ b/src/services/events/types.ts @@ -1,15 +1,29 @@ -// Add new events to the union type here. -// Please keep the values of `type` very generic as we have a limited number of -// them. Instead, add more detail in `properties` where possible. -// Adding event types this way provides type safety for names and event +import { Provider } from 'shared/api/helpers' + +// +// Add new events to the the Event union type below! +// +// Adding event types in this way provides type safety for names and event // properties. // E.g., every 'Button Clicked' event must have the buttonType property. +// +// Guidelines: +// - Event names should: +// - be of the form "[Noun] [Past-tense verb]", +// - have each word capitalized, +// - describe an action taken by the user. +// - Keep the values of `type` very generic as we have a limited number of +// them. Instead, add more detail in `properties` where possible. +// - Try to keep event property names unique to the event type to avoid +// accidental correlation of unrelated events. +// - Never include names, only use ids. E.g., use repoid instead of repo name. +// export type Event = | { type: 'Button Clicked' properties: { - buttonType: 'Install Github App' | 'Configure Repo' + buttonType: 'Install GitHub App' | 'Configure Repo' buttonLocation?: string } } @@ -19,3 +33,44 @@ export type Event = pageName: 'OwnerPage' } } + +export type Identity = { + userOwnerId: number + provider: Provider +} + +// Describes the context to be attached to events. We can extend this as needs +// arise in the future. +export type EventContext = { + // owner the event is being performed ON, not BY. + owner?: { + id: number + } + repo?: { + id: number + isPrivate?: boolean + } +} + +export abstract class EventTracker { + // Identifies the user this session belongs to. + identify(_identity: Identity): void { + throw new Error( + 'EventTracker is abstract. Method identify must be implemented.' + ) + } + + // Tracks an event + track(_event: Event): void { + throw new Error( + 'EventTracker is abstract. Method track must be implemented.' + ) + } + + // Sets the current EventContext + setContext(_context: EventContext): void { + throw new Error( + 'EventTracker is abstract. Method setContext must be implemented.' + ) + } +} diff --git a/src/services/repo/useRepo.tsx b/src/services/repo/useRepo.tsx index 382d028f04..d55010d040 100644 --- a/src/services/repo/useRepo.tsx +++ b/src/services/repo/useRepo.tsx @@ -11,6 +11,7 @@ import { const RepositorySchema = z.object({ __typename: z.literal('Repository'), + repoid: z.number(), private: z.boolean().nullable(), uploadToken: z.string().nullable(), defaultBranch: z.string().nullable(), @@ -47,6 +48,7 @@ const query = ` repository(name: $repo) { __typename ... on Repository { + repoid private uploadToken defaultBranch @@ -73,6 +75,7 @@ type UseRepoArgs = { repo: string opts?: { refetchOnWindowFocus?: boolean + enabled?: boolean } } diff --git a/src/services/user/useOwner.ts b/src/services/user/useOwner.ts index 1ab74ca5f1..3274504a39 100644 --- a/src/services/user/useOwner.ts +++ b/src/services/user/useOwner.ts @@ -73,7 +73,7 @@ export function useOwner({ }) } - return res?.data?.owner + return parsedRes?.data?.owner }), ...opts, }) diff --git a/src/services/user/useUser.ts b/src/services/user/useUser.ts index 733bb0a277..5ce9c657c0 100644 --- a/src/services/user/useUser.ts +++ b/src/services/user/useUser.ts @@ -167,9 +167,9 @@ export function useUser({ options }: UseUserArgs = {}) { parsedRes.data.me?.owner.ownerid && parsedRes.data.me?.owner.username ) { - eventTracker(provider).identify({ + eventTracker().identify({ userOwnerId: parsedRes.data.me.owner.ownerid, - username: parsedRes.data.me.owner.username, + provider, }) } diff --git a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx index 577774b759..05c5aa48fd 100644 --- a/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx +++ b/src/shared/ListRepo/InactiveRepo/InactiveRepo.tsx @@ -1,7 +1,4 @@ -import { useParams } from 'react-router' - import { eventTracker } from 'services/events/events' -import { Provider } from 'shared/api/helpers' import A from 'ui/A' function InactiveRepo({ @@ -15,7 +12,6 @@ function InactiveRepo({ repoName?: string isCurrentUserPartOfOrg?: boolean }) { - const { provider } = useParams<{ provider: Provider }>() if (isActive) return <>Deactivated if (!isCurrentUserPartOfOrg) return <>Inactive @@ -32,10 +28,11 @@ function InactiveRepo({ }, }} onClick={() => - eventTracker(provider, owner, repoName).track({ + eventTracker().track({ type: 'Button Clicked', properties: { buttonType: 'Configure Repo', + buttonLocation: 'Repo list', }, }) } diff --git a/src/ui/ContextSwitcher/ContextSwitcher.tsx b/src/ui/ContextSwitcher/ContextSwitcher.tsx index 283382ce64..d853ea3cb6 100644 --- a/src/ui/ContextSwitcher/ContextSwitcher.tsx +++ b/src/ui/ContextSwitcher/ContextSwitcher.tsx @@ -219,11 +219,11 @@ function ContextSwitcher({ - eventTracker(provider, owner).track({ + eventTracker().track({ type: 'Button Clicked', properties: { - buttonType: 'Install Github App', - buttonLocation: 'ContextSwitcher', + buttonType: 'Install GitHub App', + buttonLocation: 'Org selector', }, }) } From 3069c0fbdc79506f95c663fa614aee35b567bd62 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Mon, 13 Jan 2025 16:44:45 -0500 Subject: [PATCH 5/5] Couple small fixes --- src/layouts/Header/components/UserDropdown/UserDropdown.tsx | 2 -- .../HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx | 4 ++-- src/services/events/amplitude/amplitude.ts | 4 ++-- src/services/events/types.ts | 2 +- src/services/user/useUser.ts | 1 - 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx index e9e5a4aa60..73061d7cac 100644 --- a/src/layouts/Header/components/UserDropdown/UserDropdown.tsx +++ b/src/layouts/Header/components/UserDropdown/UserDropdown.tsx @@ -12,8 +12,6 @@ import { Dropdown } from 'ui/Dropdown/Dropdown' interface URLParams { provider: Provider - owner: string - repo?: string } type DropdownItem = { diff --git a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx index 6aba1c0ef4..780b05639e 100644 --- a/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx +++ b/src/pages/OwnerPage/HeaderBanners/GithubConfigBanner/GithubConfigBanner.jsx @@ -8,7 +8,7 @@ import BannerContent from 'ui/Banner/BannerContent' import BannerHeading from 'ui/Banner/BannerHeading' const GithubConfigBanner = () => { - const { provider, owner } = useParams() + const { provider } = useParams() const isGh = providerToName(provider) === 'GitHub' if (!isGh) return null @@ -23,7 +23,7 @@ const GithubConfigBanner = () => { data-testid="codecovGithubApp-link" to={{ pageName: 'codecovGithubAppSelectTarget' }} onClick={() => - eventTracker(provider, owner).track({ + eventTracker().track({ type: 'Button Clicked', properties: { buttonType: 'Install GitHub App', diff --git a/src/services/events/amplitude/amplitude.ts b/src/services/events/amplitude/amplitude.ts index 35f00553d6..207d3e62dd 100644 --- a/src/services/events/amplitude/amplitude.ts +++ b/src/services/events/amplitude/amplitude.ts @@ -46,10 +46,10 @@ export class AmplitudeEventTracker implements EventTracker { ...event.properties, ...this.context, }, - // This attaches the event to the owner's user group as well + // This attaches the event to the owner's user group (org) as well groups: this.context.owner?.id ? { - owner: this.context.owner.id, + org: this.context.owner.id, } : undefined, }) diff --git a/src/services/events/types.ts b/src/services/events/types.ts index 2cf9baa571..4eb18ffd98 100644 --- a/src/services/events/types.ts +++ b/src/services/events/types.ts @@ -30,7 +30,7 @@ export type Event = | { type: 'Page Viewed' properties: { - pageName: 'OwnerPage' + pageName: 'Owner Page' } } diff --git a/src/services/user/useUser.ts b/src/services/user/useUser.ts index 5ce9c657c0..179b45541c 100644 --- a/src/services/user/useUser.ts +++ b/src/services/user/useUser.ts @@ -127,7 +127,6 @@ fragment CurrentUserFragment on Me { interface URLParams { provider: Provider - owner: string } interface UseUserArgs {