Merge 7566d79 into 1dc534a

visumickey · web-flow · commit 1b29fc7d70ba · 2020-05-19T22:09:50.000-07:00
diff --git a/packages/performance/src/services/remote_config_service.test.ts b/packages/performance/src/services/remote_config_service.test.ts
@@ -20,7 +20,7 @@ import { SettingsService } from './settings_service';
 import { CONFIG_EXPIRY_LOCAL_STORAGE_KEY } from '../constants';
 import { setupApi, Api } from './api_service';
 import * as iidService from './iid_service';
-import { getConfig, isDestFl } from './remote_config_service';
+import { getConfig } from './remote_config_service';
 import { FirebaseApp } from '@firebase/app-types';
 import '../../test/setup';
 
@@ -37,7 +37,6 @@ describe('Performance Monitoring > remote_config_service', () => {
   "fpr_log_endpoint_url":"https://firebaselogging.test.com",\
   "fpr_log_transport_key":"pseudo-transport-key",\
   "fpr_log_source":"2","fpr_vc_network_request_sampling_rate":"0.250000",\
-  "fpr_log_transport_web_percent":"100.0",\
   "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
   "state":"UPDATE"}`;
   const PROJECT_ID = 'project1';
@@ -135,7 +134,6 @@ describe('Performance Monitoring > remote_config_service', () => {
         TRANSPORT_KEY
       );
       expect(SettingsService.getInstance().logSource).to.equal(LOG_SOURCE);
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.true;
       expect(
         SettingsService.getInstance().networkRequestsSamplingRate
       ).to.equal(NETWORK_SAMPLIG_RATE);
@@ -176,7 +174,6 @@ describe('Performance Monitoring > remote_config_service', () => {
         TRANSPORT_KEY
       );
       expect(SettingsService.getInstance().logSource).to.equal(LOG_SOURCE);
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.true;
       expect(
         SettingsService.getInstance().networkRequestsSamplingRate
       ).to.equal(NETWORK_SAMPLIG_RATE);
@@ -240,131 +237,38 @@ describe('Performance Monitoring > remote_config_service', () => {
       expect(SettingsService.getInstance().loggingEnabled).to.be.true;
     });
 
-    it('marks event destination to cc if there is no template', async () => {
-      setup(
-        {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
-        },
-        { reject: false, value: new Response('{"state":"NO_TEMPLATE"}') }
-      );
-      await getConfig(IID);
-
-      // If no template, will send to cc.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.false;
-    });
-
-    it('marks event destination to cc if instance state unspecified', async () => {
-      setup(
-        {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
-        },
-        {
-          reject: false,
-          value: new Response('{"state":"INSTANCE_STATE_UNSPECIFIED"}')
-        }
-      );
-      await getConfig(IID);
-
-      // If instance state unspecified, will send to cc.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.false;
-    });
+    it('gets the config from RC even with deprecated transport flag', async () => {
+      // Expired local config.
+      const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895320';
+      const STRINGIFIED_CUSTOM_CONFIG = `{"entries":{\
+        "fpr_vc_network_request_sampling_rate":"0.250000",\
+        "fpr_log_transport_web_percent":"100.0",\
+        "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
+        "state":"UPDATE"}`;
 
-    it("marks event destination to cc if state doesn't exist", async () => {
-      setup(
+      const { storageGetItemStub: getItemStub } = setup(
         {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
+          expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+          config: STRINGIFIED_CUSTOM_CONFIG
         },
-        { reject: false, value: new Response('{}') }
+        { reject: false, value: new Response(STRINGIFIED_CONFIG) }
       );
-      await getConfig(IID);
 
-      // If "state" doesn't exist, will send to cc.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.false;
-    });
-
-    it('marks event destination to Fl if template exists but no rollout flag', async () => {
-      const CONFIG_WITHOUT_ROLLOUT_FLAG = `{"entries":{"fpr_enabled":"true",\
-    "fpr_log_endpoint_url":"https://firebaselogging.test.com",\
-    "fpr_log_source":"2","fpr_vc_network_request_sampling_rate":"0.250000",\
-    "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
-    "state":"UPDATE"}`;
-      setup(
-        {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
-        },
-        { reject: false, value: new Response(CONFIG_WITHOUT_ROLLOUT_FLAG) }
-      );
       await getConfig(IID);
 
-      // If template exists but no rollout flag, will send to Fl.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.true;
-    });
-
-    it('marks event destination to cc when instance is outside of rollout range', async () => {
-      const CONFIG_WITH_ROLLOUT_FLAG_10 = `{"entries":{"fpr_enabled":"true",\
-    "fpr_log_endpoint_url":"https://firebaselogging.test.com",\
-    "fpr_log_source":"2","fpr_vc_network_request_sampling_rate":"0.250000",\
-    "fpr_log_transport_web_percent":"10.0",\
-    "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
-    "state":"UPDATE"}`;
-      setup(
-        {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
-        },
-        { reject: false, value: new Response(CONFIG_WITH_ROLLOUT_FLAG_10) }
+      expect(getItemStub).to.be.calledOnce;
+      expect(SettingsService.getInstance().loggingEnabled).to.be.true;
+      expect(SettingsService.getInstance().logEndPointUrl).to.equal(LOG_URL);
+      expect(SettingsService.getInstance().transportKey).to.equal(
+        TRANSPORT_KEY
       );
-      await getConfig(IID);
-
-      // If rollout flag exists, will send to cc when this instance is out of rollout scope.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.false;
-    });
-
-    it('marks event destination to Fl when instance is within rollout range', async () => {
-      const CONFIG_WITH_ROLLOUT_FLAG_40 = `{"entries":{"fpr_enabled":"true",\
-    "fpr_log_endpoint_url":"https://firebaselogging.test.com",\
-    "fpr_log_source":"2","fpr_vc_network_request_sampling_rate":"0.250000",\
-    "fpr_log_transport_web_percent":"40.0",\
-    "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
-    "state":"UPDATE"}`;
-      setup(
-        {
-          // Expired local config.
-          expiry: '1556524895320',
-          config: NOT_VALID_CONFIG
-        },
-        { reject: false, value: new Response(CONFIG_WITH_ROLLOUT_FLAG_40) }
+      expect(SettingsService.getInstance().logSource).to.equal(LOG_SOURCE);
+      expect(
+        SettingsService.getInstance().networkRequestsSamplingRate
+      ).to.equal(NETWORK_SAMPLIG_RATE);
+      expect(SettingsService.getInstance().tracesSamplingRate).to.equal(
+        TRACE_SAMPLING_RATE
       );
-      await getConfig(IID);
-
-      // If rollout flag exists, will send to Fl when this instance is within rollout scope.
-      expect(SettingsService.getInstance().shouldSendToFl).to.be.true;
-    });
-  });
-
-  describe('isDestFl', () => {
-    it('marks traffic to cc when rollout percentage is 0', () => {
-      const shouldSendToFl = isDestFl('abc', 0); // Hash percentage of "abc" is 38%.
-      expect(shouldSendToFl).to.be.false;
-    });
-
-    it('marks traffic to Fl when rollout percentage is 100', () => {
-      const shouldSendToFl = isDestFl('abc', 100); // Hash percentage of "abc" is 38%.
-      expect(shouldSendToFl).to.be.true;
-    });
-
-    it('marks traffic to Fl if hash percentage is lower than rollout percentage 50%', () => {
-      const shouldSendToFl = isDestFl('abc', 50); // Hash percentage of "abc" is 38%.
-      expect(shouldSendToFl).to.be.true;
     });
   });
 });
diff --git a/packages/performance/src/services/remote_config_service.ts b/packages/performance/src/services/remote_config_service.ts
@@ -34,22 +34,14 @@ interface SecondaryConfig {
   logSource?: number;
   logEndPointUrl?: string;
   transportKey?: string;
-  shouldSendToFl?: boolean;
   tracesSamplingRate?: number;
   networkRequestsSamplingRate?: number;
 }
 
 // These values will be used if the remote config object is successfully
 // retrieved, but the template does not have these fields.
 const DEFAULT_CONFIGS: SecondaryConfig = {
-  loggingEnabled: true,
-  shouldSendToFl: true
-};
-
-// These values will be used if the remote config object is successfully
-// retrieved, but the config object state shows unspecified or no template.
-const NO_TEMPLATE_CONFIGS: SecondaryConfig = {
-  shouldSendToFl: false
+  loggingEnabled: true
 };
 
 /* eslint-disable camelcase */
@@ -75,12 +67,12 @@ const FIS_AUTH_PREFIX = 'FIREBASE_INSTALLATIONS_AUTH';
 export function getConfig(iid: string): Promise<void> {
   const config = getStoredConfig();
   if (config) {
-    processConfig(iid, config);
+    processConfig(config);
     return Promise.resolve();
   }
 
   return getRemoteConfig(iid)
-    .then(config => processConfig(iid, config))
+    .then(processConfig)
     .then(
       config => storeConfig(config),
       /** Do nothing for error, use defaults set in settings service. */
@@ -170,15 +162,13 @@ function getRemoteConfig(
  * is valid.
  */
 function processConfig(
-  iid: string,
-  config: RemoteConfigResponse | undefined
+  config?: RemoteConfigResponse
 ): RemoteConfigResponse | undefined {
   if (!config) {
     return config;
   }
   const settingsServiceInstance = SettingsService.getInstance();
   const entries = config.entries || {};
-  const state = config.state;
   if (entries.fpr_enabled !== undefined) {
     // TODO: Change the assignment of loggingEnabled once the received type is
     // known.
@@ -208,32 +198,6 @@ function processConfig(
     settingsServiceInstance.transportKey = DEFAULT_CONFIGS.transportKey;
   }
 
-  // If config object state indicates that no template has been set, that means it is new user of
-  // Performance Monitoring and should use the old log endpoint, because it is guaranteed to work.
-  if (
-    state === undefined ||
-    state === 'INSTANCE_STATE_UNSPECIFIED' ||
-    state === 'NO_TEMPLATE'
-  ) {
-    if (NO_TEMPLATE_CONFIGS.shouldSendToFl !== undefined) {
-      settingsServiceInstance.shouldSendToFl =
-        NO_TEMPLATE_CONFIGS.shouldSendToFl;
-    }
-  } else if (entries.fpr_log_transport_web_percent) {
-    // If config object state doesn't indicate no template, it can only be UPDATE for now.
-    // - Performance Monitoring doesn't set etag in request, therefore state cannot be NO_CHANGE.
-    // - Sampling rate flags and master flag are required, therefore state cannot be EMPTY_CONFIG.
-    // If config object state is UPDATE and rollout flag is present, determine endpoint by iid.
-    settingsServiceInstance.shouldSendToFl = isDestFl(
-      iid,
-      Number(entries.fpr_log_transport_web_percent)
-    );
-  } else if (DEFAULT_CONFIGS.shouldSendToFl !== undefined) {
-    // If config object state is UPDATE and rollout flag is not present, that means rollout is
-    // complete and rollout flag is deprecated, therefore dispatch events to new transport endpoint.
-    settingsServiceInstance.shouldSendToFl = DEFAULT_CONFIGS.shouldSendToFl;
-  }
-
   if (entries.fpr_vc_network_request_sampling_rate !== undefined) {
     settingsServiceInstance.networkRequestsSamplingRate = Number(
       entries.fpr_vc_network_request_sampling_rate
@@ -267,30 +231,3 @@ function configValid(expiry: string): boolean {
 function shouldLogAfterSampling(samplingRate: number): boolean {
   return Math.random() <= samplingRate;
 }
-
-/**
- * True if event should be sent to Fl transport endpoint rather than CC transport endpoint.
- * rolloutPercent is in range [0.0, 100.0].
- * @param iid Installation ID which identifies a web app installed on client.
- * @param rolloutPercent the possibility of this app sending events to Fl endpoint.
- * @return true if this installation should send events to Fl endpoint.
- */
-export function isDestFl(iid: string, rolloutPercent: number): boolean {
-  if (iid.length === 0) {
-    return false;
-  }
-  return getHashPercent(iid) < rolloutPercent;
-}
-/**
- * Generate integer value range in [0, 99]. Implementation from String.hashCode() in Java.
- * @param seed Same seed will generate consistent hash value using this algorithm.
- * @return Hash value in range [0, 99], generated from seed and hash algorithm.
- */
-function getHashPercent(seed: string): number {
-  let hash = 0;
-  for (let i = 0; i < seed.length; i++) {
-    hash = (hash << 3) + hash - seed.charCodeAt(i);
-  }
-  hash = Math.abs(hash % 100);
-  return hash;
-}
diff --git a/packages/performance/src/services/settings_service.ts b/packages/performance/src/services/settings_service.ts
@@ -47,8 +47,6 @@ export class SettingsService {
 
   transportKey = mergeStrings('AzSC8r6ReiGqFMyfvgow', 'Iayx0u-XT3vksVM-pIV');
 
-  shouldSendToFl = false;
-
   // Source type for performance event logs.
   logSource = 462;
 
diff --git a/packages/performance/src/services/transport_service.test.ts b/packages/performance/src/services/transport_service.test.ts
@@ -47,7 +47,6 @@ describe('Firebase Performance > transport_service', () => {
     fetchStub.restore();
     clock.restore();
     resetTransportService();
-    SettingsService.getInstance().shouldSendToFl = false;
   });
 
   it('throws an error when logging an empty message', () => {
@@ -87,7 +86,6 @@ describe('Firebase Performance > transport_service', () => {
     const setting = SettingsService.getInstance();
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
-    setting.shouldSendToFl = true;
     fetchStub.withArgs(flTransportFullUrl, match.any).resolves(
       // DELETE_REQUEST means event dispatch is successful.
       new Response(
diff --git a/packages/performance/src/services/transport_service.ts b/packages/performance/src/services/transport_service.ts
@@ -112,7 +112,7 @@ function dispatchQueueEvents(): void {
   };
   /* eslint-enable camelcase */
 
-  postToEndpoint(data, staged).catch(() => {
+  sendEventsToFl(data, staged).catch(() => {
     // If the request fails for some reason, add the events that were attempted
     // back to the primary queue to retry later.
     queue = [...staged, ...queue];
@@ -122,18 +122,6 @@ function dispatchQueueEvents(): void {
   });
 }
 
-function postToEndpoint(
-  data: TransportBatchLogFormat,
-  staged: BatchEvent[]
-): Promise<void> {
-  // Gradually rollout traffic from cc to transport using remote config.
-  if (SettingsService.getInstance().shouldSendToFl) {
-    return sendEventsToFl(data, staged);
-  } else {
-    return sendEventsToCc(data);
-  }
-}
-
 function sendEventsToFl(
   data: TransportBatchLogFormat,
   staged: BatchEvent[]
@@ -171,29 +159,6 @@ function sendEventsToFl(
     });
 }
 
-function sendEventsToCc(data: TransportBatchLogFormat): Promise<void> {
-  return fetch(SettingsService.getInstance().logEndPointUrl, {
-    method: 'POST',
-    body: JSON.stringify(data)
-  })
-    .then(res => {
-      if (!res.ok) {
-        consoleLogger.info('Call to Firebase backend failed.');
-      }
-      return res.json();
-    })
-    .then(res => {
-      const wait = Number(res.next_request_wait_millis);
-      // Find the next call wait time from the response.
-      const requestOffset = isNaN(wait)
-        ? DEFAULT_SEND_INTERVAL_MS
-        : Math.max(DEFAULT_SEND_INTERVAL_MS, wait);
-      remainingTries = DEFAULT_REMAINING_TRIES;
-      // Schedule the next process.
-      processQueue(requestOffset);
-    });
-}
-
 function postToFlEndpoint(data: TransportBatchLogFormat): Promise<Response> {
   const flTransportFullUrl = SettingsService.getInstance().getFlTransportFullUrl();
   return fetch(flTransportFullUrl, {
