From 7a71e2c8b3dd0e5bf03fb5fae6c1ea166b728f89 Mon Sep 17 00:00:00 2001
From: James Liu <jamxl@google.com>
Date: Sat, 3 Oct 2020 21:05:25 -0700
Subject: [PATCH 1/5] Send up to 1000 events per LogRequest for firebase
 performance

---
 .../src/services/transport_service.test.ts    | 131 +++++++++++++++---
 .../src/services/transport_service.ts         |   9 +-
 .../src/services/transport_service.test.ts    | 130 ++++++++++++++---
 .../src/services/transport_service.ts         |   9 +-
 4 files changed, 236 insertions(+), 43 deletions(-)

diff --git a/packages-exp/performance-exp/src/services/transport_service.test.ts b/packages-exp/performance-exp/src/services/transport_service.test.ts
index 871f9bb2782..fb2b98fd4dc 100644
--- a/packages-exp/performance-exp/src/services/transport_service.test.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.test.ts
@@ -31,6 +31,8 @@ describe('Firebase Performance > transport_service', () => {
   let fetchStub: SinonStub<[RequestInfo, RequestInit?], Promise<Response>>;
   const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
   const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
+  const MAX_EVENT_COUNT_PER_REQUEST = 1000;
+  const TRANSPORT_DELAY_INTERVAL = 30000;
   // Starts date at timestamp 1 instead of 0, otherwise it causes validation errors.
   let clock: SinonFakeTimers;
   const testTransportHandler = transportHandler((...args) => {
@@ -66,8 +68,7 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     expect(fetchStub).to.not.have.been.called;
   });
-
-  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', () => {
+  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -78,36 +79,128 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     testTransportHandler('someEvent');
     clock.tick(DEFAULT_SEND_INTERVAL_MS);
+    await Promise.resolve();
     expect(fetchStub).to.have.been.calledOnce;
   });
 
   it('successful send a meesage to transport', () => {
-    const transportDelayInterval = 30000;
     const setting = SettingsService.getInstance();
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
     fetchStub.withArgs(flTransportFullUrl, match.any).resolves(
       // DELETE_REQUEST means event dispatch is successful.
-      new Response(
-        '{\
-        "nextRequestWaitMillis": "' +
-          transportDelayInterval +
-          '",\
-        "logResponseDetails": [\
-          {\
-            "responseAction": "DELETE_REQUEST"\
-          }\
-        ]\
-      }',
-        {
-          status: 200,
-          headers: { 'Content-type': 'application/json' }
-        }
-      )
+      generateSuccessResponse()
     );
 
     testTransportHandler('event1');
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     expect(fetchStub).to.have.been.calledOnce;
   });
+
+  it('sends up to the maximum event limit in one request', async () => {
+    // Arrange
+    const setting = SettingsService.getInstance();
+    const flTransportFullUrl =
+      setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
+    // Generates the first logRequest which contains first 1000 events.
+    const firstLogRequest = generateLogRequest('5501');
+    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
+      firstLogRequest['log_event'].push({
+        'source_extension_json_proto3': 'event' + i,
+        'event_time_ms': '1'
+      });
+    }
+    // Generates the second logRequest which contains remaining 20 events;
+    const secondLogRequest = generateLogRequest('35504');
+    for (let i = 0; i < 20; i++) {
+      secondLogRequest['log_event'].push({
+        'source_extension_json_proto3':
+          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
+        'event_time_ms': '1'
+      });
+    }
+
+    // Returns successful response from fl for logRequests.
+    const response = generateSuccessResponse();
+    fetchStub
+      .withArgs(flTransportFullUrl, {
+        method: 'POST',
+        body: JSON.stringify(firstLogRequest)
+      })
+      .resolves(response);
+    fetchStub
+      .withArgs(flTransportFullUrl, {
+        method: 'POST',
+        body: JSON.stringify(secondLogRequest)
+      })
+      .resolves(response);
+    stub(response, 'json').returns(
+      Promise.resolve(JSON.parse(generateSuccessResponseBody()))
+    );
+
+    // Act
+    // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
+    for (let i = 0; i < 1020; i++) {
+      testTransportHandler('event' + i);
+    }
+
+    // Assert
+    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    // First logRequest has been called.
+    expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(firstLogRequest)
+    });
+    // Wait for async action to call for next dispatch cycle.
+    await Promise.resolve()
+      .then(() => {
+        clock.tick(1);
+      })
+      .then(() => {
+        clock.tick(1);
+      })
+      .then(() => {
+        clock.tick(1);
+      });
+    clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
+    // Second logRequest has been called.
+    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(secondLogRequest)
+    });
+  });
+
+  function generateLogRequest(requestTimeMs: string): any {
+    return {
+      'request_time_ms': requestTimeMs,
+      'client_info': {
+        'client_type': 1,
+        'js_client_info': {}
+      },
+      'log_source': 462,
+      'log_event': [] as any
+    };
+  }
+
+  function generateSuccessResponse(): Response {
+    return new Response(generateSuccessResponseBody(), {
+      status: 200,
+      headers: { 'Content-type': 'application/json' }
+    });
+  }
+
+  function generateSuccessResponseBody(): string {
+    return (
+      '{\
+    "nextRequestWaitMillis": "' +
+      TRANSPORT_DELAY_INTERVAL +
+      '",\
+    "logResponseDetails": [\
+      {\
+        "responseAction": "DELETE_REQUEST"\
+      }\
+    ]\
+    }'
+    );
+  }
 });
diff --git a/packages-exp/performance-exp/src/services/transport_service.ts b/packages-exp/performance-exp/src/services/transport_service.ts
index cc1328fe637..432a8ac914d 100644
--- a/packages-exp/performance-exp/src/services/transport_service.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.ts
@@ -23,6 +23,7 @@ const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
 const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
 // If end point does not work, the call will be tried for these many times.
 const DEFAULT_REMAINING_TRIES = 3;
+const MAX_EVENT_COUNT_PER_REQUEST = 1000;
 let remainingTries = DEFAULT_REMAINING_TRIES;
 
 interface LogResponseDetails {
@@ -90,9 +91,11 @@ function processQueue(timeOffset: number): void {
 }
 
 function dispatchQueueEvents(): void {
-  // Capture a snapshot of the queue and empty the "official queue".
-  const staged = [...queue];
-  queue = [];
+  // Extract events up to the maximum cap of single logRequest from top of "official queue".
+  // The staged events will be used for current logRequest attempt, remaining events will be kept
+  // for next attempt.
+  const staged = queue.slice(0, MAX_EVENT_COUNT_PER_REQUEST);
+  queue = queue.slice(staged.length);
 
   /* eslint-disable camelcase */
   // We will pass the JSON serialized event to the backend.
diff --git a/packages/performance/src/services/transport_service.test.ts b/packages/performance/src/services/transport_service.test.ts
index 7f9a64594ed..b1384d67561 100644
--- a/packages/performance/src/services/transport_service.test.ts
+++ b/packages/performance/src/services/transport_service.test.ts
@@ -31,6 +31,8 @@ describe('Firebase Performance > transport_service', () => {
   let fetchStub: SinonStub<[RequestInfo, RequestInit?], Promise<Response>>;
   const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
   const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
+  const MAX_EVENT_COUNT_PER_REQUEST = 1000;
+  const TRANSPORT_DELAY_INTERVAL = 30000;
   // Starts date at timestamp 1 instead of 0, otherwise it causes validation errors.
   let clock: SinonFakeTimers;
   const testTransportHandler = transportHandler((...args) => {
@@ -67,7 +69,7 @@ describe('Firebase Performance > transport_service', () => {
     expect(fetchStub).to.not.have.been.called;
   });
 
-  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', () => {
+  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -78,36 +80,128 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     testTransportHandler('someEvent');
     clock.tick(DEFAULT_SEND_INTERVAL_MS);
+    await Promise.resolve();
     expect(fetchStub).to.have.been.calledOnce;
   });
 
   it('successful send a meesage to transport', () => {
-    const transportDelayInterval = 30000;
     const setting = SettingsService.getInstance();
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
     fetchStub.withArgs(flTransportFullUrl, match.any).resolves(
       // DELETE_REQUEST means event dispatch is successful.
-      new Response(
-        '{\
-        "nextRequestWaitMillis": "' +
-          transportDelayInterval +
-          '",\
-        "logResponseDetails": [\
-          {\
-            "responseAction": "DELETE_REQUEST"\
-          }\
-        ]\
-      }',
-        {
-          status: 200,
-          headers: { 'Content-type': 'application/json' }
-        }
-      )
+      generateSuccessResponse()
     );
 
     testTransportHandler('event1');
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     expect(fetchStub).to.have.been.calledOnce;
   });
+
+  it('sends up to the maximum event limit in one request', async () => {
+    // Arrange
+    const setting = SettingsService.getInstance();
+    const flTransportFullUrl =
+      setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
+    // Generates the first logRequest which contains first 1000 events.
+    const firstLogRequest = generateLogRequest('5501');
+    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
+      firstLogRequest['log_event'].push({
+        'source_extension_json_proto3': 'event' + i,
+        'event_time_ms': '1'
+      });
+    }
+    // Generates the second logRequest which contains remaining 20 events;
+    const secondLogRequest = generateLogRequest('35504');
+    for (let i = 0; i < 20; i++) {
+      secondLogRequest['log_event'].push({
+        'source_extension_json_proto3':
+          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
+        'event_time_ms': '1'
+      });
+    }
+
+    // Returns successful response from fl for logRequests.
+    const response = generateSuccessResponse();
+    fetchStub
+      .withArgs(flTransportFullUrl, {
+        method: 'POST',
+        body: JSON.stringify(firstLogRequest)
+      })
+      .resolves(response);
+    fetchStub
+      .withArgs(flTransportFullUrl, {
+        method: 'POST',
+        body: JSON.stringify(secondLogRequest)
+      })
+      .resolves(response);
+    stub(response, 'json').returns(
+      Promise.resolve(JSON.parse(generateSuccessResponseBody()))
+    );
+
+    // Act
+    // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
+    for (let i = 0; i < 1020; i++) {
+      testTransportHandler('event' + i);
+    }
+
+    // Assert
+    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    // First logRequest has been called.
+    expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(firstLogRequest)
+    });
+    // Wait for async action to call for next dispatch cycle.
+    await Promise.resolve()
+      .then(() => {
+        clock.tick(1);
+      })
+      .then(() => {
+        clock.tick(1);
+      })
+      .then(() => {
+        clock.tick(1);
+      });
+    clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
+    // Second logRequest has been called.
+    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(secondLogRequest)
+    });
+  });
+
+  function generateLogRequest(requestTimeMs: string): any {
+    return {
+      'request_time_ms': requestTimeMs,
+      'client_info': {
+        'client_type': 1,
+        'js_client_info': {}
+      },
+      'log_source': 462,
+      'log_event': [] as any
+    };
+  }
+
+  function generateSuccessResponse(): Response {
+    return new Response(generateSuccessResponseBody(), {
+      status: 200,
+      headers: { 'Content-type': 'application/json' }
+    });
+  }
+
+  function generateSuccessResponseBody(): string {
+    return (
+      '{\
+    "nextRequestWaitMillis": "' +
+      TRANSPORT_DELAY_INTERVAL +
+      '",\
+    "logResponseDetails": [\
+      {\
+        "responseAction": "DELETE_REQUEST"\
+      }\
+    ]\
+    }'
+    );
+  }
 });
diff --git a/packages/performance/src/services/transport_service.ts b/packages/performance/src/services/transport_service.ts
index 5447a77e21c..79f1697ba8a 100644
--- a/packages/performance/src/services/transport_service.ts
+++ b/packages/performance/src/services/transport_service.ts
@@ -23,6 +23,7 @@ const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
 const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
 // If end point does not work, the call will be tried for these many times.
 const DEFAULT_REMAINING_TRIES = 3;
+const MAX_EVENT_COUNT_PER_REQUEST = 1000;
 let remainingTries = DEFAULT_REMAINING_TRIES;
 
 interface LogResponseDetails {
@@ -90,9 +91,11 @@ function processQueue(timeOffset: number): void {
 }
 
 function dispatchQueueEvents(): void {
-  // Capture a snapshot of the queue and empty the "official queue".
-  const staged = [...queue];
-  queue = [];
+  // Extract events up to the maximum cap of single logRequest from top of "official queue".
+  // The staged events will be used for current logRequest attempt, remaining events will be kept
+  // for next attempt.
+  const staged = queue.slice(0, MAX_EVENT_COUNT_PER_REQUEST);
+  queue = queue.slice(staged.length);
 
   /* eslint-disable camelcase */
   // We will pass the JSON serialized event to the backend.

From 7d84d1c8261af99304c4eed59dfc88ddb8e11215 Mon Sep 17 00:00:00 2001
From: James Liu <jamxl@google.com>
Date: Sat, 3 Oct 2020 21:13:44 -0700
Subject: [PATCH 2/5] Add changeset

---
 .changeset/snake-ten-some.md | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 .changeset/snake-ten-some.md

diff --git a/.changeset/snake-ten-some.md b/.changeset/snake-ten-some.md
new file mode 100644
index 00000000000..4dc2fefea28
--- /dev/null
+++ b/.changeset/snake-ten-some.md
@@ -0,0 +1,6 @@
+---
+'@firebase/performance': patch
+'firebase': patch
+---
+
+Dispatch up to 1000 events for each network request when collecting performance events.

From 3d584f6e0218788bdba6cfe1414ea4f6690d13f6 Mon Sep 17 00:00:00 2001
From: James Liu <jamxl@google.com>
Date: Sun, 18 Oct 2020 02:33:46 -0700
Subject: [PATCH 3/5] Use splice instead of slice

---
 .../src/services/transport_service.test.ts    | 37 ++++---------------
 .../src/services/transport_service.ts         |  7 +++-
 .../src/services/transport_service.test.ts    | 37 ++++---------------
 .../src/services/transport_service.ts         |  7 +++-
 4 files changed, 26 insertions(+), 62 deletions(-)

diff --git a/packages-exp/performance-exp/src/services/transport_service.test.ts b/packages-exp/performance-exp/src/services/transport_service.test.ts
index fb2b98fd4dc..1dafa685e0d 100644
--- a/packages-exp/performance-exp/src/services/transport_service.test.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.test.ts
@@ -21,7 +21,8 @@ import * as sinonChai from 'sinon-chai';
 import {
   transportHandler,
   setupTransportService,
-  resetTransportService
+  resetTransportService,
+  readQueue
 } from './transport_service';
 import { SettingsService } from './settings_service';
 
@@ -79,7 +80,6 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     testTransportHandler('someEvent');
     clock.tick(DEFAULT_SEND_INTERVAL_MS);
-    await Promise.resolve();
     expect(fetchStub).to.have.been.calledOnce;
   });
 
@@ -103,35 +103,20 @@ describe('Firebase Performance > transport_service', () => {
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
     // Generates the first logRequest which contains first 1000 events.
-    const firstLogRequest = generateLogRequest('5501');
+    const logRequest = generateLogRequest('5501');
     for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
-      firstLogRequest['log_event'].push({
+      logRequest['log_event'].push({
         'source_extension_json_proto3': 'event' + i,
         'event_time_ms': '1'
       });
     }
-    // Generates the second logRequest which contains remaining 20 events;
-    const secondLogRequest = generateLogRequest('35504');
-    for (let i = 0; i < 20; i++) {
-      secondLogRequest['log_event'].push({
-        'source_extension_json_proto3':
-          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
-        'event_time_ms': '1'
-      });
-    }
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
     fetchStub
       .withArgs(flTransportFullUrl, {
         method: 'POST',
-        body: JSON.stringify(firstLogRequest)
-      })
-      .resolves(response);
-    fetchStub
-      .withArgs(flTransportFullUrl, {
-        method: 'POST',
-        body: JSON.stringify(secondLogRequest)
+        body: JSON.stringify(logRequest)
       })
       .resolves(response);
     stub(response, 'json').returns(
@@ -149,25 +134,19 @@ describe('Firebase Performance > transport_service', () => {
     // First logRequest has been called.
     expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
       method: 'POST',
-      body: JSON.stringify(firstLogRequest)
+      body: JSON.stringify(logRequest)
     });
     // Wait for async action to call for next dispatch cycle.
     await Promise.resolve()
       .then(() => {
         clock.tick(1);
       })
-      .then(() => {
-        clock.tick(1);
-      })
       .then(() => {
         clock.tick(1);
       });
     clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
-    // Second logRequest has been called.
-    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
-      method: 'POST',
-      body: JSON.stringify(secondLogRequest)
-    });
+    // Remaining events stay in queue.
+    expect(readQueue().length).to.be.equal(20);
   });
 
   function generateLogRequest(requestTimeMs: string): any {
diff --git a/packages-exp/performance-exp/src/services/transport_service.ts b/packages-exp/performance-exp/src/services/transport_service.ts
index 432a8ac914d..275ddb296b6 100644
--- a/packages-exp/performance-exp/src/services/transport_service.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.ts
@@ -66,6 +66,10 @@ export function setupTransportService(): void {
   }
 }
 
+export function readQueue(): BatchEvent[] {
+  return [...queue];
+}
+
 /**
  * Utilized by testing to clean up message queue and un-initialize transport service.
  */
@@ -94,8 +98,7 @@ function dispatchQueueEvents(): void {
   // Extract events up to the maximum cap of single logRequest from top of "official queue".
   // The staged events will be used for current logRequest attempt, remaining events will be kept
   // for next attempt.
-  const staged = queue.slice(0, MAX_EVENT_COUNT_PER_REQUEST);
-  queue = queue.slice(staged.length);
+  const staged = queue.splice(0, MAX_EVENT_COUNT_PER_REQUEST);
 
   /* eslint-disable camelcase */
   // We will pass the JSON serialized event to the backend.
diff --git a/packages/performance/src/services/transport_service.test.ts b/packages/performance/src/services/transport_service.test.ts
index b1384d67561..e3c0f6d856e 100644
--- a/packages/performance/src/services/transport_service.test.ts
+++ b/packages/performance/src/services/transport_service.test.ts
@@ -21,7 +21,8 @@ import * as sinonChai from 'sinon-chai';
 import {
   transportHandler,
   setupTransportService,
-  resetTransportService
+  resetTransportService,
+  readQueue
 } from './transport_service';
 import { SettingsService } from './settings_service';
 
@@ -80,7 +81,6 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     testTransportHandler('someEvent');
     clock.tick(DEFAULT_SEND_INTERVAL_MS);
-    await Promise.resolve();
     expect(fetchStub).to.have.been.calledOnce;
   });
 
@@ -104,35 +104,20 @@ describe('Firebase Performance > transport_service', () => {
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
     // Generates the first logRequest which contains first 1000 events.
-    const firstLogRequest = generateLogRequest('5501');
+    const logRequest = generateLogRequest('5501');
     for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
-      firstLogRequest['log_event'].push({
+      logRequest['log_event'].push({
         'source_extension_json_proto3': 'event' + i,
         'event_time_ms': '1'
       });
     }
-    // Generates the second logRequest which contains remaining 20 events;
-    const secondLogRequest = generateLogRequest('35504');
-    for (let i = 0; i < 20; i++) {
-      secondLogRequest['log_event'].push({
-        'source_extension_json_proto3':
-          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
-        'event_time_ms': '1'
-      });
-    }
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
     fetchStub
       .withArgs(flTransportFullUrl, {
         method: 'POST',
-        body: JSON.stringify(firstLogRequest)
-      })
-      .resolves(response);
-    fetchStub
-      .withArgs(flTransportFullUrl, {
-        method: 'POST',
-        body: JSON.stringify(secondLogRequest)
+        body: JSON.stringify(logRequest)
       })
       .resolves(response);
     stub(response, 'json').returns(
@@ -150,25 +135,19 @@ describe('Firebase Performance > transport_service', () => {
     // First logRequest has been called.
     expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
       method: 'POST',
-      body: JSON.stringify(firstLogRequest)
+      body: JSON.stringify(logRequest)
     });
     // Wait for async action to call for next dispatch cycle.
     await Promise.resolve()
       .then(() => {
         clock.tick(1);
       })
-      .then(() => {
-        clock.tick(1);
-      })
       .then(() => {
         clock.tick(1);
       });
     clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
-    // Second logRequest has been called.
-    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
-      method: 'POST',
-      body: JSON.stringify(secondLogRequest)
-    });
+    // Remaining events stay in queue.
+    expect(readQueue().length).to.be.equal(20);
   });
 
   function generateLogRequest(requestTimeMs: string): any {
diff --git a/packages/performance/src/services/transport_service.ts b/packages/performance/src/services/transport_service.ts
index 79f1697ba8a..a8df279b904 100644
--- a/packages/performance/src/services/transport_service.ts
+++ b/packages/performance/src/services/transport_service.ts
@@ -66,6 +66,10 @@ export function setupTransportService(): void {
   }
 }
 
+export function readQueue(): BatchEvent[] {
+  return [...queue];
+}
+
 /**
  * Utilized by testing to clean up message queue and un-initialize transport service.
  */
@@ -94,8 +98,7 @@ function dispatchQueueEvents(): void {
   // Extract events up to the maximum cap of single logRequest from top of "official queue".
   // The staged events will be used for current logRequest attempt, remaining events will be kept
   // for next attempt.
-  const staged = queue.slice(0, MAX_EVENT_COUNT_PER_REQUEST);
-  queue = queue.slice(staged.length);
+  const staged = queue.splice(0, MAX_EVENT_COUNT_PER_REQUEST);
 
   /* eslint-disable camelcase */
   // We will pass the JSON serialized event to the backend.

From fee0229c95f91c56d571c18eb09dbb551cdf0d2e Mon Sep 17 00:00:00 2001
From: James Liu <jamxl@google.com>
Date: Sun, 1 Nov 2020 22:47:32 -0800
Subject: [PATCH 4/5] Simplified tests for better readability

---
 .../src/services/transport_service.test.ts    | 64 +++++++++----------
 .../src/services/transport_service.ts         |  4 --
 .../src/services/transport_service.test.ts    | 64 +++++++++----------
 .../src/services/transport_service.ts         |  4 --
 4 files changed, 60 insertions(+), 76 deletions(-)

diff --git a/packages-exp/performance-exp/src/services/transport_service.test.ts b/packages-exp/performance-exp/src/services/transport_service.test.ts
index 1dafa685e0d..2ae35cba5a9 100644
--- a/packages-exp/performance-exp/src/services/transport_service.test.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.test.ts
@@ -21,8 +21,7 @@ import * as sinonChai from 'sinon-chai';
 import {
   transportHandler,
   setupTransportService,
-  resetTransportService,
-  readQueue
+  resetTransportService
 } from './transport_service';
 import { SettingsService } from './settings_service';
 
@@ -33,7 +32,7 @@ describe('Firebase Performance > transport_service', () => {
   const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
   const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
   const MAX_EVENT_COUNT_PER_REQUEST = 1000;
-  const TRANSPORT_DELAY_INTERVAL = 30000;
+  const TRANSPORT_DELAY_INTERVAL = 10000;
   // Starts date at timestamp 1 instead of 0, otherwise it causes validation errors.
   let clock: SinonFakeTimers;
   const testTransportHandler = transportHandler((...args) => {
@@ -102,51 +101,48 @@ describe('Firebase Performance > transport_service', () => {
     const setting = SettingsService.getInstance();
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
-    // Generates the first logRequest which contains first 1000 events.
-    const logRequest = generateLogRequest('5501');
-    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
-      logRequest['log_event'].push({
-        'source_extension_json_proto3': 'event' + i,
-        'event_time_ms': '1'
-      });
-    }
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
-    fetchStub
-      .withArgs(flTransportFullUrl, {
-        method: 'POST',
-        body: JSON.stringify(logRequest)
-      })
-      .resolves(response);
-    stub(response, 'json').returns(
-      Promise.resolve(JSON.parse(generateSuccessResponseBody()))
-    );
+    fetchStub.resolves(response);
+    stub(response, 'json').resolves(JSON.parse(generateSuccessResponseBody()));
 
     // Act
     // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
     for (let i = 0; i < 1020; i++) {
       testTransportHandler('event' + i);
     }
+    // Wait for first and second event dispatch to happen.
+    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    await Promise.resolve().then().then().then();
+    clock.tick(DEFAULT_SEND_INTERVAL_MS);
 
     // Assert
-    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
-    // First logRequest has been called.
-    expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
+    // Expects the first logRequest which contains first 1000 events.
+    const firstLogRequest = generateLogRequest('5501');
+    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
+      firstLogRequest['log_event'].push({
+        'source_extension_json_proto3': 'event' + i,
+        'event_time_ms': '1'
+      });
+    }
+    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
       method: 'POST',
-      body: JSON.stringify(logRequest)
+      body: JSON.stringify(firstLogRequest)
     });
-    // Wait for async action to call for next dispatch cycle.
-    await Promise.resolve()
-      .then(() => {
-        clock.tick(1);
-      })
-      .then(() => {
-        clock.tick(1);
+    // Expects the second logRequest which contains remaining 20 events;
+    const secondLogRequest = generateLogRequest('15501');
+    for (let i = 0; i < 20; i++) {
+      secondLogRequest['log_event'].push({
+        'source_extension_json_proto3':
+          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
+        'event_time_ms': '1'
       });
-    clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
-    // Remaining events stay in queue.
-    expect(readQueue().length).to.be.equal(20);
+    }
+    expect(fetchStub).calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(secondLogRequest)
+    });
   });
 
   function generateLogRequest(requestTimeMs: string): any {
diff --git a/packages-exp/performance-exp/src/services/transport_service.ts b/packages-exp/performance-exp/src/services/transport_service.ts
index 275ddb296b6..87f8efab773 100644
--- a/packages-exp/performance-exp/src/services/transport_service.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.ts
@@ -66,10 +66,6 @@ export function setupTransportService(): void {
   }
 }
 
-export function readQueue(): BatchEvent[] {
-  return [...queue];
-}
-
 /**
  * Utilized by testing to clean up message queue and un-initialize transport service.
  */
diff --git a/packages/performance/src/services/transport_service.test.ts b/packages/performance/src/services/transport_service.test.ts
index e3c0f6d856e..e83a9cf9008 100644
--- a/packages/performance/src/services/transport_service.test.ts
+++ b/packages/performance/src/services/transport_service.test.ts
@@ -21,8 +21,7 @@ import * as sinonChai from 'sinon-chai';
 import {
   transportHandler,
   setupTransportService,
-  resetTransportService,
-  readQueue
+  resetTransportService
 } from './transport_service';
 import { SettingsService } from './settings_service';
 
@@ -33,7 +32,7 @@ describe('Firebase Performance > transport_service', () => {
   const INITIAL_SEND_TIME_DELAY_MS = 5.5 * 1000;
   const DEFAULT_SEND_INTERVAL_MS = 10 * 1000;
   const MAX_EVENT_COUNT_PER_REQUEST = 1000;
-  const TRANSPORT_DELAY_INTERVAL = 30000;
+  const TRANSPORT_DELAY_INTERVAL = 10000;
   // Starts date at timestamp 1 instead of 0, otherwise it causes validation errors.
   let clock: SinonFakeTimers;
   const testTransportHandler = transportHandler((...args) => {
@@ -103,51 +102,48 @@ describe('Firebase Performance > transport_service', () => {
     const setting = SettingsService.getInstance();
     const flTransportFullUrl =
       setting.flTransportEndpointUrl + '?key=' + setting.transportKey;
-    // Generates the first logRequest which contains first 1000 events.
-    const logRequest = generateLogRequest('5501');
-    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
-      logRequest['log_event'].push({
-        'source_extension_json_proto3': 'event' + i,
-        'event_time_ms': '1'
-      });
-    }
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
-    fetchStub
-      .withArgs(flTransportFullUrl, {
-        method: 'POST',
-        body: JSON.stringify(logRequest)
-      })
-      .resolves(response);
-    stub(response, 'json').returns(
-      Promise.resolve(JSON.parse(generateSuccessResponseBody()))
-    );
+    fetchStub.resolves(response);
+    stub(response, 'json').resolves(JSON.parse(generateSuccessResponseBody()));
 
     // Act
     // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
     for (let i = 0; i < 1020; i++) {
       testTransportHandler('event' + i);
     }
+    // Wait for first and second event dispatch to happen.
+    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    await Promise.resolve().then().then().then();
+    clock.tick(DEFAULT_SEND_INTERVAL_MS);
 
     // Assert
-    clock.tick(INITIAL_SEND_TIME_DELAY_MS);
-    // First logRequest has been called.
-    expect(fetchStub).to.have.been.calledWith(flTransportFullUrl, {
+    // Expects the first logRequest which contains first 1000 events.
+    const firstLogRequest = generateLogRequest('5501');
+    for (let i = 0; i < MAX_EVENT_COUNT_PER_REQUEST; i++) {
+      firstLogRequest['log_event'].push({
+        'source_extension_json_proto3': 'event' + i,
+        'event_time_ms': '1'
+      });
+    }
+    expect(fetchStub).which.to.have.been.calledWith(flTransportFullUrl, {
       method: 'POST',
-      body: JSON.stringify(logRequest)
+      body: JSON.stringify(firstLogRequest)
     });
-    // Wait for async action to call for next dispatch cycle.
-    await Promise.resolve()
-      .then(() => {
-        clock.tick(1);
-      })
-      .then(() => {
-        clock.tick(1);
+    // Expects the second logRequest which contains remaining 20 events;
+    const secondLogRequest = generateLogRequest('15501');
+    for (let i = 0; i < 20; i++) {
+      secondLogRequest['log_event'].push({
+        'source_extension_json_proto3':
+          'event' + (MAX_EVENT_COUNT_PER_REQUEST + i),
+        'event_time_ms': '1'
       });
-    clock.tick(DEFAULT_SEND_INTERVAL_MS * 3);
-    // Remaining events stay in queue.
-    expect(readQueue().length).to.be.equal(20);
+    }
+    expect(fetchStub).calledWith(flTransportFullUrl, {
+      method: 'POST',
+      body: JSON.stringify(secondLogRequest)
+    });
   });
 
   function generateLogRequest(requestTimeMs: string): any {
diff --git a/packages/performance/src/services/transport_service.ts b/packages/performance/src/services/transport_service.ts
index a8df279b904..d57d8f5cf72 100644
--- a/packages/performance/src/services/transport_service.ts
+++ b/packages/performance/src/services/transport_service.ts
@@ -66,10 +66,6 @@ export function setupTransportService(): void {
   }
 }
 
-export function readQueue(): BatchEvent[] {
-  return [...queue];
-}
-
 /**
  * Utilized by testing to clean up message queue and un-initialize transport service.
  */

From 2b979e05f1a831256f2d36fac482bb46bdfa706d Mon Sep 17 00:00:00 2001
From: alikn <alikashefian@google.com>
Date: Mon, 7 Dec 2020 16:44:42 -0800
Subject: [PATCH 5/5] Minor clean up

---
 .../src/services/transport_service.test.ts                | 8 +++++---
 .../performance/src/services/transport_service.test.ts    | 7 ++++---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/packages-exp/performance-exp/src/services/transport_service.test.ts b/packages-exp/performance-exp/src/services/transport_service.test.ts
index 2ae35cba5a9..43986e00817 100644
--- a/packages-exp/performance-exp/src/services/transport_service.test.ts
+++ b/packages-exp/performance-exp/src/services/transport_service.test.ts
@@ -57,7 +57,7 @@ describe('Firebase Performance > transport_service', () => {
     }).to.throw;
   });
 
-  it('does not attempt to log an event to cc after INITIAL_SEND_TIME_DELAY_MS if queue is empty', () => {
+  it('does not attempt to log an event after INITIAL_SEND_TIME_DELAY_MS if queue is empty', () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -68,7 +68,8 @@ describe('Firebase Performance > transport_service', () => {
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
     expect(fetchStub).to.not.have.been.called;
   });
-  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
+
+  it('attempts to log an event after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -104,8 +105,8 @@ describe('Firebase Performance > transport_service', () => {
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
-    fetchStub.resolves(response);
     stub(response, 'json').resolves(JSON.parse(generateSuccessResponseBody()));
+    fetchStub.resolves(response);
 
     // Act
     // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
@@ -114,6 +115,7 @@ describe('Firebase Performance > transport_service', () => {
     }
     // Wait for first and second event dispatch to happen.
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    // This is to resolve the floating promise chain in transport service.
     await Promise.resolve().then().then().then();
     clock.tick(DEFAULT_SEND_INTERVAL_MS);
 
diff --git a/packages/performance/src/services/transport_service.test.ts b/packages/performance/src/services/transport_service.test.ts
index e83a9cf9008..caa1958aad4 100644
--- a/packages/performance/src/services/transport_service.test.ts
+++ b/packages/performance/src/services/transport_service.test.ts
@@ -57,7 +57,7 @@ describe('Firebase Performance > transport_service', () => {
     }).to.throw;
   });
 
-  it('does not attempt to log an event to cc after INITIAL_SEND_TIME_DELAY_MS if queue is empty', () => {
+  it('does not attempt to log an event after INITIAL_SEND_TIME_DELAY_MS if queue is empty', () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -69,7 +69,7 @@ describe('Firebase Performance > transport_service', () => {
     expect(fetchStub).to.not.have.been.called;
   });
 
-  it('attempts to log an event to cc after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
+  it('attempts to log an event after DEFAULT_SEND_INTERVAL_MS if queue not empty', async () => {
     fetchStub.resolves(
       new Response('', {
         status: 200,
@@ -105,8 +105,8 @@ describe('Firebase Performance > transport_service', () => {
 
     // Returns successful response from fl for logRequests.
     const response = generateSuccessResponse();
-    fetchStub.resolves(response);
     stub(response, 'json').resolves(JSON.parse(generateSuccessResponseBody()));
+    fetchStub.resolves(response);
 
     // Act
     // Generate 1020 events, which should be dispatched in two batches (1000 events and 20 events).
@@ -115,6 +115,7 @@ describe('Firebase Performance > transport_service', () => {
     }
     // Wait for first and second event dispatch to happen.
     clock.tick(INITIAL_SEND_TIME_DELAY_MS);
+    // This is to resolve the floating promise chain in transport service.
     await Promise.resolve().then().then().then();
     clock.tick(DEFAULT_SEND_INTERVAL_MS);