[DevTools] RFC: Add initial APIs for logging instrumentation events u…

…nder feature flag
facebook · Sep 13, 2021 · 0ef97f4 · 0ef97f4
1 parent 33226fa
commit 0ef97f4
Show file tree

Hide file tree

Showing 13 changed files with 172 additions and 57 deletions.
diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js
@@ -18,6 +18,7 @@ import {
   localStorageRemoveItem,
   localStorageSetItem,
 } from 'react-devtools-shared/src/storage';
+import {registerEventLogger} from 'react-devtools-shared/src/Logger';
 import DevTools from 'react-devtools-shared/src/devtools/views/DevTools';
 import {__DEBUG__} from 'react-devtools-shared/src/constants';
 
@@ -87,6 +88,10 @@ function createPanelIfReactLoaded() {
 
       const tabId = chrome.devtools.inspectedWindow.tabId;
 
+      registerEventLogger((_event: LogEvent) => {
+        // TODO: hook up event logging
+      });
+
       function initBridgeAndStore() {
         const port = chrome.runtime.connect({
           name: '' + tabId,

diff --git a/packages/react-devtools-shared/src/Logger.js b/packages/react-devtools-shared/src/Logger.js
@@ -0,0 +1,43 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ */
+
+import {enableLogger} from 'react-devtools-feature-flags';
+
+export type LogEvent = {|
+  +name: 'parseHookNames',
+  +displayName: string | null,
+  +numberOfHooks: number | null,
+  +durationMs: number,
+  +resolution: 'success' | 'error' | 'timeout',
+|};
+
+export type LogFunction = LogEvent => void;
+
+let loggers = [];
+export const logEvent: LogFunction =
+  enableLogger === true
+    ? function logEvent(event: LogEvent): void {
+        loggers.forEach(log => {
+          log(event);
+        });
+      }
+    : function logEvent() {};
+
+export const registerEventLogger =
+  enableLogger === true
+    ? function registerEventLogger(eventLogger: LogFunction): () => void {
+        if (enableLogger) {
+          loggers.push(eventLogger);
+          return function unregisterEventLogger() {
+            loggers = loggers.filter(logger => logger !== eventLogger);
+          };
+        }
+        return () => {};
+      }
+    : function registerEventLogger() {};
diff --git a/...t-devtools-shared/src/PerformanceMarks.js → ...ols-shared/src/PerformanceLoggingUtils.js b/...t-devtools-shared/src/PerformanceMarks.js → ...ols-shared/src/PerformanceLoggingUtils.js
@@ -14,6 +14,9 @@ const supportsUserTiming =
   typeof performance.mark === 'function' &&
   typeof performance.clearMarks === 'function';
 
+const supportsPerformanceNow =
+  typeof performance !== 'undefined' && typeof performance.now === 'function';
+
 function mark(markName: string): void {
   if (supportsUserTiming) {
     performance.mark(markName + '-start');
@@ -27,6 +30,13 @@ function measure(markName: string): void {
   }
 }
 
+function now(): number {
+  if (supportsPerformanceNow) {
+    return performance.now();
+  }
+  return Date.now();
+}
+
 export async function withAsyncPerformanceMark<TReturn>(
   markName: string,
   callback: () => Promise<TReturn>,
@@ -66,3 +76,32 @@ export function withCallbackPerformanceMark<TReturn>(
   }
   return callback(() => {});
 }
+
+export async function measureAsyncDuration<TReturn>(
+  callback: () => Promise<TReturn>,
+): Promise<[number, TReturn]> {
+  const start = now();
+  const result = await callback();
+  const duration = now() - start;
+  return [duration, result];
+}
+
+export function measureSyncDuration<TReturn>(
+  callback: () => TReturn,
+): [number, TReturn] {
+  const start = now();
+  const result = callback();
+  const duration = now() - start;
+  return [duration, result];
+}
+
+export function measureCallbackDuration<TReturn>(
+  callback: (done: () => number) => TReturn,
+): TReturn {
+  const start = now();
+  const done = () => {
+    const duration = now() - start;
+    return duration;
+  };
+  return callback(done);
+}
diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js
@@ -16,6 +16,8 @@
 export const enableProfilerChangedHookIndices = true;
 export const isInternalFacebookBuild = true;
 export const enableNamedHooksFeature = false;
+export const enableLogger = false;
+
 export const consoleManagedByDevToolsDuringStrictMode = false;
 
 /************************************************************************

diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js
@@ -16,6 +16,8 @@
 export const enableProfilerChangedHookIndices = false;
 export const isInternalFacebookBuild = false;
 export const enableNamedHooksFeature = false;
+export const enableLogger = false;
+
 export const consoleManagedByDevToolsDuringStrictMode = false;
 
 /************************************************************************

diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js
@@ -16,4 +16,6 @@
 export const enableProfilerChangedHookIndices = false;
 export const isInternalFacebookBuild = false;
 export const enableNamedHooksFeature = true;
+export const enableLogger = false;
+
 export const consoleManagedByDevToolsDuringStrictMode = true;
diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js
@@ -16,6 +16,8 @@
 export const enableProfilerChangedHookIndices = true;
 export const isInternalFacebookBuild = true;
 export const enableNamedHooksFeature = true;
+export const enableLogger = false;
+
 export const consoleManagedByDevToolsDuringStrictMode = true;
 
 /************************************************************************

diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js
@@ -16,6 +16,8 @@
 export const enableProfilerChangedHookIndices = true;
 export const isInternalFacebookBuild = false;
 export const enableNamedHooksFeature = true;
+export const enableLogger = false;
+
 export const consoleManagedByDevToolsDuringStrictMode = true;
 
 /************************************************************************

diff --git a/packages/react-devtools-shared/src/hookNamesCache.js b/packages/react-devtools-shared/src/hookNamesCache.js
@@ -18,6 +18,8 @@ import type {
 } from 'react-devtools-shared/src/types';
 import type {HookSource} from 'react-debug-tools/src/ReactDebugHooks';
 import type {FetchFileWithCaching} from 'react-devtools-shared/src/devtools/views/Components/FetchFileWithCachingContext';
+import {measureCallbackDuration} from './PerformanceLoggingUtils';
+import {logEvent} from './Logger';
 
 const TIMEOUT = 30000;
 
@@ -92,7 +94,13 @@ export function loadHookNames(
       },
     };
 
-    const wake = () => {
+    let timeoutID;
+
+    const wake = (
+      resolution: 'success' | 'error' | 'timeout',
+      hookNames: HookNames | null,
+      durationMs: number,
+    ) => {
       if (timeoutID) {
         clearTimeout(timeoutID);
         timeoutID = null;
@@ -101,6 +109,15 @@ export function loadHookNames(
       // This assumes they won't throw.
       callbacks.forEach(callback => callback());
       callbacks.clear();
+
+      // Log duration for parsing hook names
+      logEvent({
+        name: 'parseHookNames',
+        displayName: element.displayName,
+        numberOfHooks: hookNames?.size ?? null,
+        durationMs,
+        resolution,
+      });
     };
 
     const newRecord: Record<HookNames> = (record = {
@@ -110,64 +127,65 @@ export function loadHookNames(
 
     let didTimeout = false;
 
-    loadHookNamesFunction(hooksTree, fetchFileWithCaching).then(
-      function onSuccess(hookNames) {
-        if (didTimeout) {
-          return;
-        }
-
+    measureCallbackDuration(done => {
+      loadHookNamesFunction(hooksTree, fetchFileWithCaching).then(
+        function onSuccess(hookNames) {
+          if (didTimeout) {
+            return;
+          }
+
+          if (__DEBUG__) {
+            console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
+          }
+
+          if (hookNames) {
+            const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
+            resolvedRecord.status = Resolved;
+            resolvedRecord.value = hookNames;
+          } else {
+            const notFoundRecord = ((newRecord: any): RejectedRecord);
+            notFoundRecord.status = Rejected;
+            notFoundRecord.value = null;
+          }
+
+          wake('success', hookNames, done());
+        },
+        function onError(error) {
+          if (didTimeout) {
+            return;
+          }
+
+          if (__DEBUG__) {
+            console.log('[hookNamesCache] onError()');
+          }
+
+          console.error(error);
+
+          const thrownRecord = ((newRecord: any): RejectedRecord);
+          thrownRecord.status = Rejected;
+          thrownRecord.value = null;
+
+          wake('error', null, done());
+        },
+      );
+
+      // Eventually timeout and stop trying to load names.
+      timeoutID = setTimeout(function onTimeout() {
         if (__DEBUG__) {
-          console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
-        }
-
-        if (hookNames) {
-          const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
-          resolvedRecord.status = Resolved;
-          resolvedRecord.value = hookNames;
-        } else {
-          const notFoundRecord = ((newRecord: any): RejectedRecord);
-          notFoundRecord.status = Rejected;
-          notFoundRecord.value = null;
+          console.log('[hookNamesCache] onTimeout()');
         }
 
-        wake();
-      },
-      function onError(error) {
-        if (didTimeout) {
-          return;
-        }
-
-        if (__DEBUG__) {
-          console.log('[hookNamesCache] onError()');
-        }
-
-        console.error(error);
-
-        const thrownRecord = ((newRecord: any): RejectedRecord);
-        thrownRecord.status = Rejected;
-        thrownRecord.value = null;
-
-        wake();
-      },
-    );
-
-    // Eventually timeout and stop trying to load names.
-    let timeoutID = setTimeout(function onTimeout() {
-      if (__DEBUG__) {
-        console.log('[hookNamesCache] onTimeout()');
-      }
-
-      timeoutID = null;
-
-      didTimeout = true;
+        timeoutID = null;
 
-      const timedoutRecord = ((newRecord: any): RejectedRecord);
-      timedoutRecord.status = Rejected;
-      timedoutRecord.value = null;
+        didTimeout = true;
 
-      wake();
-    }, TIMEOUT);
+        const timedoutRecord = ((newRecord: any): RejectedRecord);
+        timedoutRecord.status = Rejected;
+        timedoutRecord.value = null;
 
+        wake('timeout', null, done());
+      }, TIMEOUT);
+    });
     map.set(element, record);
   }
 

diff --git a/packages/react-devtools-shared/src/hooks/astUtils.js b/packages/react-devtools-shared/src/hooks/astUtils.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import {withSyncPerformanceMark} from 'react-devtools-shared/src/PerformanceMarks';
+import {withSyncPerformanceMark} from 'react-devtools-shared/src/PerformanceLoggingUtils';
 import traverse, {NodePath, Node} from '@babel/traverse';
 import {File} from '@babel/types';
 

diff --git a/packages/react-devtools-shared/src/hooks/parseHookNames/index.js b/packages/react-devtools-shared/src/hooks/parseHookNames/index.js
@@ -12,7 +12,7 @@ import type {HooksNode, HooksTree} from 'react-debug-tools/src/ReactDebugHooks';
 import type {HookNames} from 'react-devtools-shared/src/types';
 import type {FetchFileWithCaching} from 'react-devtools-shared/src/devtools/views/Components/FetchFileWithCachingContext';
 
-import {withAsyncPerformanceMark} from 'react-devtools-shared/src/PerformanceMarks';
+import {withAsyncPerformanceMark} from 'react-devtools-shared/src/PerformanceLoggingUtils';
 import WorkerizedParseSourceAndMetadata from './parseSourceAndMetadata.worker';
 import typeof * as ParseSourceAndMetadataModule from './parseSourceAndMetadata';
 import {flattenHooksList, loadSourceAndMetadata} from './loadSourceAndMetadata';

diff --git a/packages/react-devtools-shared/src/hooks/parseHookNames/loadSourceAndMetadata.js b/packages/react-devtools-shared/src/hooks/parseHookNames/loadSourceAndMetadata.js
@@ -52,7 +52,7 @@ import {
   withAsyncPerformanceMark,
   withCallbackPerformanceMark,
   withSyncPerformanceMark,
-} from 'react-devtools-shared/src/PerformanceMarks';
+} from 'react-devtools-shared/src/PerformanceLoggingUtils';
 
 import type {
   HooksNode,

diff --git a/packages/react-devtools-shared/src/hooks/parseHookNames/parseSourceAndMetadata.js b/packages/react-devtools-shared/src/hooks/parseHookNames/parseSourceAndMetadata.js
@@ -21,7 +21,7 @@ import {SourceMapMetadataConsumer} from '../SourceMapMetadataConsumer';
 import {
   withAsyncPerformanceMark,
   withSyncPerformanceMark,
-} from 'react-devtools-shared/src/PerformanceMarks';
+} from 'react-devtools-shared/src/PerformanceLoggingUtils';
 
 import type {
   HooksList,