From 980b14089151ecb82f4abe76fc96307141d7ff16 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Tue, 10 Nov 2020 09:49:01 -0600 Subject: [PATCH] track sensitiveProperties in a duplicate event. (#9807) --- shared/modules/metametrics.js | 26 ++++++++- .../swaps/awaiting-swap/awaiting-swap.js | 8 +-- ui/app/pages/swaps/index.js | 8 +-- .../loading-swaps-quotes.js | 8 +-- ui/app/pages/swaps/view-quote/view-quote.js | 55 +++++-------------- 5 files changed, 42 insertions(+), 63 deletions(-) diff --git a/shared/modules/metametrics.js b/shared/modules/metametrics.js index cfe59248c..1a564dab4 100644 --- a/shared/modules/metametrics.js +++ b/shared/modules/metametrics.js @@ -1,5 +1,5 @@ import Analytics from 'analytics-node' -import { omit, pick } from 'lodash' +import { merge, omit, pick } from 'lodash' // flushAt controls how many events are sent to segment at once. Segment // will hold onto a queue of events until it hits this number, then it sends @@ -137,6 +137,8 @@ export const segmentLegacy = * @property {string} category - category to associate event to * @property {boolean} [isOptIn] - happened during opt in/out workflow * @property {object} [properties] - object of custom values to track, snake_case + * @property {object} [sensitiveProperties] - Object of sensitive values to track, snake_case. + * These properties will be sent in an additional event that excludes the user's metaMetricsId. * @property {number} [revenue] - amount of currency that event creates in revenue for MetaMask * @property {string} [currency] - ISO 4127 format currency for events with revenue, defaults to US dollars * @property {number} [value] - Abstract "value" that this event has for MetaMask. @@ -168,6 +170,7 @@ export function getTrackMetaMetricsEvent(metamaskVersion, getDynamicState) { category, isOptIn, properties = {}, + sensitiveProperties, revenue, currency, value, @@ -179,6 +182,27 @@ export function getTrackMetaMetricsEvent(metamaskVersion, getDynamicState) { if (!event || !category) { throw new Error('Must specify event and category.') } + // Uses recursion to track a duplicate event with sensitive properties included, + // but metaMetricsId excluded + if (sensitiveProperties) { + if (excludeId === true) { + throw new Error( + 'sensitiveProperties was specified in an event payload that also set the excludeMetaMetricsId flag', + ) + } + trackMetaMetricsEvent({ + event, + category, + isOptIn, + properties: merge(sensitiveProperties, properties), + revenue, + currency, + value, + excludeMetaMetricsId: true, + matomoEvent, + eventContext, + }) + } const { participateInMetaMetrics, context: providedContext, diff --git a/ui/app/pages/swaps/awaiting-swap/awaiting-swap.js b/ui/app/pages/swaps/awaiting-swap/awaiting-swap.js index 252acc00c..7e2430d63 100644 --- a/ui/app/pages/swaps/awaiting-swap/awaiting-swap.js +++ b/ui/app/pages/swaps/awaiting-swap/awaiting-swap.js @@ -94,8 +94,7 @@ export default function AwaitingSwap({ const quotesExpiredEvent = useNewMetricEvent({ event: 'Quotes Timed Out', - excludeMetaMetricsId: true, - properties: { + sensitiveProperties: { token_from: sourceTokenInfo?.symbol, token_from_amount: fetchParams?.value, token_to: destinationTokenInfo?.symbol, @@ -106,10 +105,6 @@ export default function AwaitingSwap({ }, category: 'swaps', }) - const anonymousQuotesExpiredEvent = useNewMetricEvent({ - event: 'Quotes Timed Out', - category: 'swaps', - }) const blockExplorerUrl = txHash && getBlockExplorerUrlForTx(networkId, txHash, rpcPrefs) @@ -196,7 +191,6 @@ export default function AwaitingSwap({ if (!trackedQuotesExpiredEvent) { setTrackedQuotesExpiredEvent(true) quotesExpiredEvent() - anonymousQuotesExpiredEvent() } } else if (errorKey === ERROR_FETCHING_QUOTES) { headerText = t('swapFetchingQuotesErrorTitle') diff --git a/ui/app/pages/swaps/index.js b/ui/app/pages/swaps/index.js index 52522c2f1..02aff4a32 100644 --- a/ui/app/pages/swaps/index.js +++ b/ui/app/pages/swaps/index.js @@ -202,12 +202,7 @@ export default function Swap() { const exitedSwapsEvent = useNewMetricEvent({ event: 'Exited Swaps', category: 'swaps', - }) - const anonymousExitedSwapsEvent = useNewMetricEvent({ - event: 'Exited Swaps', - category: 'swaps', - excludeMetaMetricsId: true, - properties: { + sensitiveProperties: { token_from: fetchParams?.sourceTokenInfo?.symbol, token_from_amount: fetchParams?.value, request_type: fetchParams?.balanceError, @@ -221,7 +216,6 @@ export default function Swap() { useEffect(() => { exitEventRef.current = () => { exitedSwapsEvent() - anonymousExitedSwapsEvent() } }) diff --git a/ui/app/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js b/ui/app/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js index d3423a4e6..684466f27 100644 --- a/ui/app/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js +++ b/ui/app/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js @@ -69,12 +69,7 @@ export default function LoadingSwapsQuotes({ const quotesRequestCancelledEventConfig = { event: 'Quotes Request Cancelled', category: 'swaps', - } - const anonymousQuotesRequestCancelledEventConfig = { - event: 'Quotes Request Cancelled', - category: 'swaps', - excludeMetaMetricsId: true, - properties: { + sensitiveProperties: { token_from: fetchParams?.sourceTokenInfo?.symbol, token_from_amount: fetchParams?.value, request_type: fetchParams?.balanceError, @@ -232,7 +227,6 @@ export default function LoadingSwapsQuotes({ submitText={t('back')} onSubmit={async () => { metaMetricsEvent(quotesRequestCancelledEventConfig) - metaMetricsEvent(anonymousQuotesRequestCancelledEventConfig) await dispatch(navigateBackToBuildQuote(history)) }} hideCancel diff --git a/ui/app/pages/swaps/view-quote/view-quote.js b/ui/app/pages/swaps/view-quote/view-quote.js index 64b841f12..b1d7e296e 100644 --- a/ui/app/pages/swaps/view-quote/view-quote.js +++ b/ui/app/pages/swaps/view-quote/view-quote.js @@ -300,26 +300,10 @@ export default function ViewQuote() { available_quotes: numberOfQuotes, } - const anonymousAllAvailableQuotesOpened = useNewMetricEvent({ - event: 'All Available Quotes Opened', - properties: { - ...eventObjectBase, - other_quote_selected: usedQuote?.aggregator !== topQuote?.aggregator, - other_quote_selected_source: - usedQuote?.aggregator === topQuote?.aggregator - ? null - : usedQuote?.aggregator, - }, - excludeMetaMetricsId: true, - category: 'swaps', - }) const allAvailableQuotesOpened = useNewMetricEvent({ event: 'All Available Quotes Opened', category: 'swaps', - }) - const anonymousQuoteDetailsOpened = useNewMetricEvent({ - event: 'Quote Details Opened', - properties: { + sensitiveProperties: { ...eventObjectBase, other_quote_selected: usedQuote?.aggregator !== topQuote?.aggregator, other_quote_selected_source: @@ -327,38 +311,34 @@ export default function ViewQuote() { ? null : usedQuote?.aggregator, }, - excludeMetaMetricsId: true, - category: 'swaps', }) const quoteDetailsOpened = useNewMetricEvent({ event: 'Quote Details Opened', category: 'swaps', + sensitiveProperties: { + ...eventObjectBase, + other_quote_selected: usedQuote?.aggregator !== topQuote?.aggregator, + other_quote_selected_source: + usedQuote?.aggregator === topQuote?.aggregator + ? null + : usedQuote?.aggregator, + }, }) - const anonymousEditSpendLimitOpened = useNewMetricEvent({ + const editSpendLimitOpened = useNewMetricEvent({ event: 'Edit Spend Limit Opened', - properties: { + category: 'swaps', + sensitiveProperties: { ...eventObjectBase, custom_spend_limit_set: originalApproveAmount === approveAmount, custom_spend_limit_amount: originalApproveAmount === approveAmount ? null : approveAmount, }, - excludeMetaMetricsId: true, - category: 'swaps', - }) - const editSpendLimitOpened = useNewMetricEvent({ - event: 'Edit Spend Limit Opened', - category: 'swaps', }) - const anonymousBestQuoteReviewedEvent = useNewMetricEvent({ - event: 'Best Quote Reviewed', - properties: { ...eventObjectBase, network_fees: feeInFiat }, - excludeMetaMetricsId: true, - category: 'swaps', - }) const bestQuoteReviewedEvent = useNewMetricEvent({ event: 'Best Quote Reviewed', category: 'swaps', + sensitiveProperties: { ...eventObjectBase, network_fees: feeInFiat }, }) useEffect(() => { if ( @@ -376,7 +356,6 @@ export default function ViewQuote() { ) { bestQuoteReviewedEventSent.current = true bestQuoteReviewedEvent() - anonymousBestQuoteReviewedEvent() } }, [ sourceTokenSymbol, @@ -388,13 +367,11 @@ export default function ViewQuote() { numberOfQuotes, feeInFiat, bestQuoteReviewedEvent, - anonymousBestQuoteReviewedEvent, ]) const metaMaskFee = usedQuote.fee const onFeeCardTokenApprovalClick = () => { - anonymousEditSpendLimitOpened() editSpendLimitOpened() dispatch( showModal({ @@ -500,10 +477,7 @@ export default function ViewQuote() { }} swapToSymbol={destinationTokenSymbol} initialAggId={usedQuote.aggregator} - onQuoteDetailsIsOpened={() => { - anonymousQuoteDetailsOpened() - quoteDetailsOpened() - }} + onQuoteDetailsIsOpened={quoteDetailsOpened} /> )}
@@ -547,7 +521,6 @@ export default function ViewQuote() {
{ - anonymousAllAvailableQuotesOpened() allAvailableQuotesOpened() setSelectQuotePopoverShown(true) }}