Fix an issue when an error is thrown for performance oob metrics (#2110)

 fix an error in the performance sdk which throws an error for oob metrics

Author: alikn

packages/performance/src/resources/trace.ts

@@ -31,7 +31,7 @@ import {
   isValidCustomAttributeName,
   isValidCustomAttributeValue
 } from '../utils/attributes_utils';
-import { isValidCustomMetricName } from '../utils/metric_utils';
+import { isValidMetricName } from '../utils/metric_utils';
 import { PerformanceTrace } from '@firebase/performance-types';
 
 const enum TraceState {
@@ -163,7 +163,7 @@ export class Trace implements PerformanceTrace {
    * @param num Set custom metric to this value
    */
   putMetric(counter: string, num: number): void {
-    if (isValidCustomMetricName(counter)) {
+    if (isValidMetricName(counter, this.name)) {
       this.counters[counter] = num;
     } else {
       throw ERROR_FACTORY.create(ErrorCode.INVALID_CUSTOM_METRIC_NAME, {
@@ -270,15 +270,15 @@ export class Trace implements PerformanceTrace {
     // navigationTimings includes only one element.
     if (navigationTimings && navigationTimings[0]) {
       trace.setDuration(Math.floor(navigationTimings[0].duration * 1000));
-      trace.incrementMetric(
+      trace.putMetric(
         'domInteractive',
         Math.floor(navigationTimings[0].domInteractive * 1000)
       );
-      trace.incrementMetric(
+      trace.putMetric(
         'domContentLoadedEventEnd',
         Math.floor(navigationTimings[0].domContentLoadedEventEnd * 1000)
       );
-      trace.incrementMetric(
+      trace.putMetric(
         'loadEventEnd',
         Math.floor(navigationTimings[0].loadEventEnd * 1000)
       );
@@ -291,7 +291,7 @@ export class Trace implements PerformanceTrace {
         paintObject => paintObject.name === FIRST_PAINT
       );
       if (firstPaint && firstPaint.startTime) {
-        trace.incrementMetric(
+        trace.putMetric(
           FIRST_PAINT_COUNTER_NAME,
           Math.floor(firstPaint.startTime * 1000)
         );
@@ -300,14 +300,14 @@ export class Trace implements PerformanceTrace {
         paintObject => paintObject.name === FIRST_CONTENTFUL_PAINT
       );
       if (firstContentfulPaint && firstContentfulPaint.startTime) {
-        trace.incrementMetric(
+        trace.putMetric(
           FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
           Math.floor(firstContentfulPaint.startTime * 1000)
         );
       }
 
       if (firstInputDelay) {
-        trace.incrementMetric(
+        trace.putMetric(
           FIRST_INPUT_DELAY_COUNTER_NAME,
           Math.floor(firstInputDelay * 1000)
         );

packages/performance/src/utils/metric_utils.test.ts

@@ -17,29 +17,77 @@
 
 import { expect } from 'chai';
 
-import { isValidCustomMetricName } from './metric_utils';
-
+import { isValidMetricName } from './metric_utils';
+import {
+  FIRST_PAINT_COUNTER_NAME,
+  FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
+  FIRST_INPUT_DELAY_COUNTER_NAME
+} from '../constants';
 import '../../test/setup';
 
 describe('Firebase Performance > metric_utils', () => {
-  describe('#isValidCustomMetricName', () => {
+  describe('#isValidMetricName', () => {
     it('returns true when name is valid', () => {
-      expect(isValidCustomMetricName('validCustom_Metric_Name')).to.be.true;
+      expect(isValidMetricName('validCustom_Metric_Name')).to.be.true;
     });
 
     it('returns false when name is blank', () => {
-      expect(isValidCustomMetricName('')).to.be.false;
+      expect(isValidMetricName('')).to.be.false;
     });
 
     it('returns false when name is too long', () => {
       const longMetricName =
         'too_long_metric_name_over_one_hundred_characters_too_long_metric_name_over_one_' +
         'hundred_characters_too';
-      expect(isValidCustomMetricName(longMetricName)).to.be.false;
+      expect(isValidMetricName(longMetricName)).to.be.false;
     });
 
     it('returns false when name starts with a reserved prefix', () => {
-      expect(isValidCustomMetricName('_invalidMetricName')).to.be.false;
+      expect(isValidMetricName('_invalidMetricName')).to.be.false;
+    });
+
+    it('returns true for first paint metric', () => {
+      expect(
+        isValidMetricName(FIRST_PAINT_COUNTER_NAME, '_wt_http://example.com')
+      ).to.be.true;
+    });
+
+    it('returns true for first contentful paint metric', () => {
+      expect(
+        isValidMetricName(
+          FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
+          '_wt_http://example.com'
+        )
+      ).to.be.true;
+    });
+
+    it('returns true for first input delay metric', () => {
+      expect(
+        isValidMetricName(
+          FIRST_INPUT_DELAY_COUNTER_NAME,
+          '_wt_http://example.com'
+        )
+      ).to.be.true;
+    });
+
+    it('returns false if first paint metric name is used outside of page load traces', () => {
+      expect(isValidMetricName(FIRST_PAINT_COUNTER_NAME, 'some_randome_trace'))
+        .to.be.false;
+    });
+
+    it('returns false if first contentful paint metric name is used outside of page load traces', () => {
+      expect(
+        isValidMetricName(
+          FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
+          'some_randome_trace'
+        )
+      ).to.be.false;
+    });
+
+    it('returns false if first input delay metric name is used outside of page load traces', () => {
+      expect(
+        isValidMetricName(FIRST_INPUT_DELAY_COUNTER_NAME, 'some_randome_trace')
+      ).to.be.false;
     });
   });
 });

packages/performance/src/utils/metric_utils.ts

@@ -15,12 +15,33 @@
  * limitations under the License.
  */
 
+import {
+  FIRST_PAINT_COUNTER_NAME,
+  FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
+  FIRST_INPUT_DELAY_COUNTER_NAME,
+  OOB_TRACE_PAGE_LOAD_PREFIX
+} from '../constants';
+
 const MAX_METRIC_NAME_LENGTH = 100;
 const RESERVED_AUTO_PREFIX = '_';
+const oobMetrics = [
+  FIRST_PAINT_COUNTER_NAME,
+  FIRST_CONTENTFUL_PAINT_COUNTER_NAME,
+  FIRST_INPUT_DELAY_COUNTER_NAME
+];
 
-export function isValidCustomMetricName(name: string): boolean {
+/**
+ * Returns true if the metric is custom and does not start with reserved prefix, or if
+ * the metric is one of out of the box page load trace metrics.
+ */
+export function isValidMetricName(name: string, traceName?: string): boolean {
   if (name.length === 0 || name.length > MAX_METRIC_NAME_LENGTH) {
     return false;
   }
-  return !name.startsWith(RESERVED_AUTO_PREFIX);
+  return (
+    (traceName &&
+      traceName.startsWith(OOB_TRACE_PAGE_LOAD_PREFIX) &&
+      oobMetrics.indexOf(name) > -1) ||
+    !name.startsWith(RESERVED_AUTO_PREFIX)
+  );
 }