From f39498703e13b54ce918866b14ebcb1e0252c8a2 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 7 Aug 2020 15:32:46 -0300 Subject: [PATCH] Use `pathname` instead of URL for `currentPath` metrics parameter (#9158) The `currentPath` parameter passed to our metrics utility had been passed the full URL rather than just the path, contrary to what the name would imply. We only used the path portion, so passing the full URL did lead to complications. Now just the `pathname` is passed in, rather than the full URL. This simplifies the metrics logic, and it incidentally fixes two bugs. The main bug fixed is regarding Firefox metrics. Previously we had assumed the `currentPath` would start with `chrome-extension://`, which of course was not true on Firefox. This lead to us incorrectly parsing the `currentPath`, so path tracking was broken for Firefox events. This broken parsing is now bypassed entirely, so metrics should now work the same on Firefox as on Chrome. The second bug was that we were incorrectly setting the tracking URL for background events during tests. As a result, we were incorrectly detecting ourselves as an internal site that had referred the user to us. But this was not of major concern, since it only affected test metrics (which get sent to the development Matomo project). Lastly, this change let us discard the `pathname` parameter used in the `overrides` parameter of the `metricsEvent` function. Now that `currentPath` is equivalent to `pathname`, the `pathname` parameter is redundant. --- app/scripts/lib/backend-metametrics.js | 8 +------- ui/app/contexts/metametrics.js | 8 ++++---- ui/app/helpers/utils/metametrics.util.js | 12 ++++++------ ui/app/pages/routes/routes.component.js | 5 +---- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/app/scripts/lib/backend-metametrics.js b/app/scripts/lib/backend-metametrics.js index 9328fa3af..e62f09d0d 100644 --- a/app/scripts/lib/backend-metametrics.js +++ b/app/scripts/lib/backend-metametrics.js @@ -1,12 +1,6 @@ import { getBackgroundMetaMetricState } from '../../../ui/app/selectors' import { sendMetaMetricsEvent } from '../../../ui/app/helpers/utils/metametrics.util' -const inDevelopment = process.env.NODE_ENV === 'development' - -const METAMETRICS_TRACKING_URL = inDevelopment - ? 'http://www.metamask.io/metametrics' - : 'http://www.metamask.io/metametrics-prod' - export default function backEndMetaMetricsEvent (metaMaskState, eventData) { const stateEventData = getBackgroundMetaMetricState({ metamask: metaMaskState }) @@ -14,7 +8,7 @@ export default function backEndMetaMetricsEvent (metaMaskState, eventData) { sendMetaMetricsEvent({ ...stateEventData, ...eventData, - currentPath: METAMETRICS_TRACKING_URL + '/background', + currentPath: '/background', }) } } diff --git a/ui/app/contexts/metametrics.js b/ui/app/contexts/metametrics.js index ced6d4944..b1440429e 100644 --- a/ui/app/contexts/metametrics.js +++ b/ui/app/contexts/metametrics.js @@ -41,7 +41,7 @@ export function MetaMetricsProvider ({ children }) { const numberOfAccounts = useSelector(getNumberOfAccounts) const history = useHistory() const [state, setState] = useState(() => ({ - currentPath: window.location.href, + currentPath: (new URL(window.location.href)).pathname, previousPath: '', })) @@ -49,7 +49,7 @@ export function MetaMetricsProvider ({ children }) { useEffect(() => { const unlisten = history.listen(() => setState((prevState) => ({ - currentPath: window.location.href, + currentPath: (new URL(window.location.href)).pathname, previousPath: prevState.currentPath, }))) // remove this listener if the component is no longer mounted @@ -59,8 +59,8 @@ export function MetaMetricsProvider ({ children }) { const metricsEvent = useCallback((config = {}, overrides = {}) => { const { eventOpts = {} } = config const { name = '' } = eventOpts - const { pathname: overRidePathName = '' } = overrides - const isSendFlow = Boolean(name.match(/^send|^confirm/) || overRidePathName.match(/send|confirm/)) + const { currentPath: overrideCurrentPath = '' } = overrides + const isSendFlow = Boolean(name.match(/^send|^confirm/) || overrideCurrentPath.match(/send|confirm/)) if (participateInMetaMetrics || config.isOptIn) { return sendMetaMetricsEvent({ diff --git a/ui/app/helpers/utils/metametrics.util.js b/ui/app/helpers/utils/metametrics.util.js index 3cba4f215..d2563c7fc 100644 --- a/ui/app/helpers/utils/metametrics.util.js +++ b/ui/app/helpers/utils/metametrics.util.js @@ -13,7 +13,7 @@ const METAMETRICS_BASE_URL = 'https://chromeextensionmm.innocraft.cloud/piwik.ph const METAMETRICS_REQUIRED_PARAMS = `?idsite=${projectId}&rec=1&apiv=1` const METAMETRICS_BASE_FULL = METAMETRICS_BASE_URL + METAMETRICS_REQUIRED_PARAMS -export const METAMETRICS_TRACKING_URL = inDevelopment +const METAMETRICS_TRACKING_URL = inDevelopment ? 'http://www.metamask.io/metametrics' : 'http://www.metamask.io/metametrics-prod' @@ -74,7 +74,7 @@ const customDimensionsNameIdMap = { function composeUrlRefParamAddition (previousPath, confirmTransactionOrigin) { const externalOrigin = confirmTransactionOrigin && confirmTransactionOrigin !== 'metamask' - return `&urlref=${externalOrigin ? 'EXTERNAL' : encodeURIComponent(previousPath.replace(/chrome-extension:\/\/\w+/, METAMETRICS_TRACKING_URL))}` + return `&urlref=${externalOrigin ? 'EXTERNAL' : encodeURIComponent(`${METAMETRICS_TRACKING_URL}${previousPath}`)}` } // composes query params of the form &dimension[0-999]=[value] @@ -115,8 +115,8 @@ function composeParamAddition (paramValue, paramName) { * @property {string} config.accountType The account type being used at the time of the event: 'hardware', 'imported' or 'default' * @property {number} config.numberOfTokens The number of tokens that the user has added at the time of the event * @property {number} config.numberOfAccounts The number of accounts the user has added at the time of the event - * @property {string} config.previousPath The location path the user was on prior to the path they are on at the time of the event - * @property {string} config.currentPath The location path the user is on at the time of the event + * @property {string} config.previousPath The pathname of the URL the user was on prior to the URL they are on at the time of the event + * @property {string} config.currentPath The pathname of the URL the user is on at the time of the event * @property {string} config.metaMetricsId A random id assigned to a user at the time of opting in to metametrics. A hexadecimal number * @property {string} config.confirmTransactionOrigin The origin on a transaction * @property {boolean} config.excludeMetaMetricsId Whether or not the tracked event data should be associated with a metametrics id @@ -165,10 +165,10 @@ function composeUrl (config) { numberOfTokens: (customVariables && customVariables.numberOfTokens) || numberOfTokens, numberOfAccounts: (customVariables && customVariables.numberOfAccounts) || numberOfAccounts, }) : '' - const url = currentPath ? `&url=${encodeURIComponent(currentPath.replace(/chrome-extension:\/\/\w+/, METAMETRICS_TRACKING_URL))}` : '' + const url = currentPath ? `&url=${encodeURIComponent(`${METAMETRICS_TRACKING_URL}${currentPath}`)}` : '' const _id = metaMetricsId && !excludeMetaMetricsId ? `&_id=${metaMetricsId.slice(2, 18)}` : '' const rand = `&rand=${String(Math.random()).slice(2)}` - const pv_id = currentPath ? `&pv_id=${ethUtil.bufferToHex(ethUtil.sha3(currentPath.match(/chrome-extension:\/\/\w+\/(.+)/)?.[0] || url)).slice(2, 8)}` : '' + const pv_id = currentPath ? `&pv_id=${ethUtil.bufferToHex(ethUtil.sha3(currentPath)).slice(2, 8)}` : '' const uid = metaMetricsId && !excludeMetaMetricsId ? `&uid=${metaMetricsId.slice(2, 18)}` : excludeMetaMetricsId diff --git a/ui/app/pages/routes/routes.component.js b/ui/app/pages/routes/routes.component.js index 1aca07f07..578483425 100644 --- a/ui/app/pages/routes/routes.component.js +++ b/ui/app/pages/routes/routes.component.js @@ -52,8 +52,6 @@ import { UNLOCK_ROUTE, } from '../../helpers/constants/routes' -import { METAMETRICS_TRACKING_URL } from '../../helpers/utils/metametrics.util' - import { ENVIRONMENT_TYPE_NOTIFICATION, ENVIRONMENT_TYPE_POPUP } from '../../../../app/scripts/lib/enums' import { getEnvironmentType } from '../../../../app/scripts/lib/util' @@ -101,8 +99,7 @@ export default class Routes extends Component { if (action === 'PUSH') { pageChanged(locationObj.pathname) this.context.metricsEvent({}, { - currentPath: `${METAMETRICS_TRACKING_URL}${locationObj.pathname}`, - pathname: locationObj.pathname, + currentPath: locationObj.pathname, pageOpts: { hideDimensions: true, },