Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DevTools] Add initial APIs for logging instrumentation events under feature flag #22276

Merged
merged 7 commits into from
Sep 14, 2021

Conversation

jstejada
Copy link
Contributor

@jstejada jstejada commented Sep 8, 2021

Summary

This commit is being used as proposal for adding a logger (Logger) that allows us to records events happening in the DevTools runtime. In particular, a version of this code was used for collecting profiling data to measure runtime the performance of parseHookNames.

API

The Logger module exposes 2 main functions:

  • registerEventLogger: This function allows the caller to register a callback that listens to any log events being emitted from the DevTools runtime.
  • logEvent: This function allows the DevTools runtime to log any relevant events. The events that can be logged are statically enforced via Flow, and they can be extended to log different types of events (e.g. other performance or usage metrics).

Both of these functions only work under the new DevTools feature flag: enableLogger, and are no-ops if the flag is not enabled.

As of this commit, the feature flag is completely disabled in every build, and the code being committed is only illustrating how the logger can be used: we are logging an event for measuring the duration for parseHookNames, and we are registering a logger when initializing the DevTools extension. The registered logger is currently a no-op, and in a follow up PR we will add an implementation that actually records those events for internal builds of DevTools only (not open source builds).

Test Plan

  • yarn flow
  • yarn test
  • yarn test-build-devtools
  • used this code to collect performance data for parseNamedHooks.
  • temporarily enable feature flag and verify events that would be logged:

image

image

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start! Thanks for the detailed right up and for committing a no-op as a starting point. That made the review a lot easier.

Added a couple of nits and thoughts, but I'm happy with this 👍🏼

packages/react-devtools-extensions/src/main.js Outdated Show resolved Hide resolved
packages/react-devtools-shared/src/Logger.js Outdated Show resolved Hide resolved
packages/react-devtools-shared/src/Logger.js Outdated Show resolved Hide resolved
@jstejada jstejada changed the title [DevTools] RFC: Add initial APIs for logging instrumentation events under feature flag [DevTools] Add initial APIs for logging instrumentation events under feature flag Sep 13, 2021
@jstejada
Copy link
Contributor Author

@bvaughn I think i addressed all of your feedback. will land unless you have additional comments

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants