From 3c171de44c76b994c2951eeecd26027994c6472f Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Tue, 3 Nov 2020 11:58:22 -0600 Subject: [PATCH] potential fix for METAMASK-GKCN (#9768) --- package.json | 2 +- shared/modules/metametrics.js | 34 +++++++++++++++++++++------------- yarn.lock | 12 ++++++------ 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 57f4d03be..bdd39471f 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "@sentry/integrations": "^5.26.0", "@zxing/library": "^0.8.0", "abortcontroller-polyfill": "^1.4.0", - "analytics-node": "^3.4.0-beta.2", + "analytics-node": "^3.4.0-beta.3", "await-semaphore": "^0.1.1", "bignumber.js": "^4.1.0", "bn.js": "^4.11.7", diff --git a/shared/modules/metametrics.js b/shared/modules/metametrics.js index f957adb8d..7bf4cec8a 100644 --- a/shared/modules/metametrics.js +++ b/shared/modules/metametrics.js @@ -1,13 +1,23 @@ import Analytics from 'analytics-node' import { omit, pick } from 'lodash' -// flushAt controls how many events are collected in the queue before they -// are sent to segment. I recommend a queue size of one due to an issue with -// detecting and flushing events in an extension beforeunload doesn't work in -// a notification context. Because notification windows are opened and closed -// in reaction to the very events we want to track, it is problematic to cache -// at all. -const flushAt = 1 +// 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 +// them as a batch. This setting defaults to 20, but that is too high for +// notification workflows. We also cannot send each event as singular payloads +// because it seems to bombard segment and potentially cause event loss. +// I chose 5 here because it is sufficiently high enough to optimize our network +// requests, while also being low enough to be reasonable. +const flushAt = process.env.METAMASK_ENVIRONMENT === 'production' ? 5 : 1 +// flushInterval controls how frequently the queue is flushed to segment. +// This happens regardless of the size of the queue. The default setting is +// 10,000ms (10 seconds). This default is absurdly high for our typical user +// flow through confirmations. I have chosen 10 ms here because it works really +// well with our wrapped track function. The track function returns a promise +// that is only fulfilled when it has been sent to segment. A 10 ms delay is +// negligible to the user, but allows us to properly batch events that happen +// in rapid succession. +const flushInterval = 10 export const METAMETRICS_ANONYMOUS_ID = '0x0000000000000000' @@ -54,11 +64,11 @@ export function sendCountIsTrackable(sendCount) { // E2E, which is handled in the build process by never providing the SEGMENT_WRITE_KEY // when process.env.IN_TEST is truthy export const segment = process.env.SEGMENT_WRITE_KEY - ? new Analytics(process.env.SEGMENT_WRITE_KEY, { flushAt }) + ? new Analytics(process.env.SEGMENT_WRITE_KEY, { flushAt, flushInterval }) : segmentNoop export const segmentLegacy = process.env.SEGMENT_LEGACY_WRITE_KEY - ? new Analytics(process.env.SEGMENT_LEGACY_WRITE_KEY, { flushAt }) + ? new Analytics(process.env.SEGMENT_LEGACY_WRITE_KEY, { flushAt, flushInterval }) : segmentNoop /** @@ -233,10 +243,7 @@ export function getTrackMetaMetricsEvent(metamaskVersion, getDynamicState) { } return new Promise((resolve, reject) => { - // This is only safe to do because we are no longer batching events through segment. - // If flushAt is greater than one the callback won't be triggered until after a number - // of events have been queued equal to the flushAt value OR flushInterval passes. The - // default flushInterval is ten seconds + // This is only safe to do because we have set an extremely low (10ms) flushInterval. const callback = (err) => { if (err) { return reject(err) @@ -249,6 +256,7 @@ export function getTrackMetaMetricsEvent(metamaskVersion, getDynamicState) { } else { segment.track(trackOptions, callback) } + }) } } diff --git a/yarn.lock b/yarn.lock index e28a423d4..fab2cfbb7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3947,13 +3947,13 @@ amdefine@>=0.0.4: resolved "https://registry.yarnpkg.com/amdefine/-/amdefine-1.0.1.tgz#4a5282ac164729e93619bcfd3ad151f817ce91f5" integrity sha1-SlKCrBZHKek2Gbz9OtFR+BfOkfU= -analytics-node@^3.4.0-beta.2: - version "3.4.0-beta.2" - resolved "https://registry.yarnpkg.com/analytics-node/-/analytics-node-3.4.0-beta.2.tgz#d4921927c2253dcc2fcbf18604dac2ee098a9b52" - integrity sha512-wjdCQQk412RBckuqGtyY7tdxaRpG7HLD0iBQrJBc7aUzHFNYyEbz3kepaUmNd5+ZbAGDqfnvKVsFO5DmzI1KNA== +analytics-node@^3.4.0-beta.3: + version "3.4.0-beta.3" + resolved "https://registry.yarnpkg.com/analytics-node/-/analytics-node-3.4.0-beta.3.tgz#5eb0694598cff493c64faf5efc1225533a253f13" + integrity sha512-NIdpxiwlZ4cKgs9MDlDe89b5bg/pMq2W7XTA+cjzCM66IwW3ujZhVE49vk+zG6Yrxk0s/DXmennJ+cCQIsTKMA== dependencies: "@segment/loosely-validate-event" "^2.0.0" - axios "^0.19.0" + axios "^0.19.2" axios-retry "^3.0.2" lodash.isstring "^4.0.1" md5 "^2.2.1" @@ -4852,7 +4852,7 @@ axios-retry@^3.0.2: dependencies: is-retry-allowed "^1.1.0" -axios@^0.19.0: +axios@^0.19.2: version "0.19.2" resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27" integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA==