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

@jstejada jstejada force-pushed the jstejada:instrumentation branch from 92796a3 to 103a03e Sep 8, 2021
@jstejada jstejada requested review from bvaughn and lunaruan Sep 8, 2021
@bvaughn
bvaughn approved these changes Sep 9, 2021
Copy link
Contributor

@bvaughn bvaughn left a comment

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 👍🏼

@jstejada jstejada force-pushed the jstejada:instrumentation branch from 103a03e to 4fdf008 Sep 13, 2021
@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

@jstejada jstejada commented Sep 13, 2021

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

Copy link
Contributor

@bvaughn bvaughn left a comment

Okay

@jstejada jstejada force-pushed the jstejada:instrumentation branch from f60b4b3 to 189a851 Sep 14, 2021
@jstejada jstejada force-pushed the jstejada:instrumentation branch from 189a851 to 665af1f Sep 14, 2021
@jstejada jstejada merged commit 50263d3 into facebook:main Sep 14, 2021
34 checks passed
34 checks passed
@facebook-github-tools
Facebook CLA Check Contributor License Agreement is valid!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_build Your tests passed on CircleCI!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_test_dom_fixtures Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_and_process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: get_base_build Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts_combined Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: sizebot Your tests passed on CircleCI!
Details
ci/circleci: sync_reconciler_forks Your tests passed on CircleCI!
Details
ci/circleci: yarn_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_build_combined Your tests passed on CircleCI!
Details
ci/circleci: yarn_flow Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=experimental --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=experimental --env=development --persistent Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=experimental --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=development --persistent Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=development --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=development --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=production --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=production --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=development --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=development --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=production --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=production --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build---project=devtools -r=experimental Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=experimental --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=experimental --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=stable --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=stable --env=production Your tests passed on CircleCI!
Details
@codesandbox
ci/codesandbox Building packages succeeded.
Details
@jstejada jstejada deleted the jstejada:instrumentation branch Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants