-
Notifications
You must be signed in to change notification settings - Fork 376
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
feat: collect bootstrap timing metrics #275
base: master
Are you sure you want to change the base?
Conversation
let perf_hooks; | ||
|
||
try { | ||
// eslint-disable-next-line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you disable the specific rule?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, but its looks like I can not disable only specific rule.
For node 6.x I should use
//eslint-disable-next-line node/no-missing-require
But for newer versions of node I should use
//eslint-disable-next-line node/no-unsupported-features/node-builtins
But I can not specify both rules. So I rolled back this to just
//eslint-disable-next-line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think this looks pretty good, just missing code for dealing with disabled timestamps
lib/metrics/bootstrapTime.js
Outdated
const now = Date.now(); | ||
|
||
if (entry.nodeStart !== -1) { | ||
nodeStart.set({}, entry.nodeStart, now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have an option for disabling timestamps - mind adding code handling it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
ef1135f
to
46daca9
Compare
d6f2cf6
to
29b6d86
Compare
Collect bootstrap timing metrics