Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
v7.x backport: src: add tracing controller#11106
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
richardlau commented Feb 1, 2017
Should #10959 be included in this or backported separately? |
joshgav commented Feb 2, 2017
There were some concerns in #9304 we wanted to address before including this in a release, in particular making the log output location configurable (#9304 (comment)). /cc @jasongin @nodejs/diagnostics |
matthewloring commented Feb 3, 2017
@joshgav The use of the @richardlau I think there are a few dependent PRs that can land if this one does. They can probably be opened separately if/once this has gone in. |
ac72c3e to 2f1ce29Compareitaloacasas commented Feb 16, 2017
@matthewloring can you rebase please |
matthewloring commented Feb 16, 2017
@italoacasas All rebased. |
1fde990 to 9c45758Comparematthewloring commented Feb 27, 2017
@joshgav Can this land? |
matthewloring commented Feb 27, 2017
It also looks like the trace event configuration options landed in the |
joshgav commented Feb 28, 2017
LGTM @italoacasas do the approvals/LGTMs from the original PR count for backports too? Or do backports need another approval? |
joshgav left a comment
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.
Rubber stamp LGTM from reviewing original PR.
italoacasas commented Feb 28, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@joshgav same process. we need LGTM here. |
italoacasas commented Feb 28, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
This is not landing in version 7 staging, because of 4b8b7e9. @matthewloring can you update the backport again please, and sorry for the inconvenience. |
This commit adds support for trace-event tracing to Node.js. It provides a mechanism to centralize tracing information generated by V8, Node core, and userspace code. It includes: - A trace writer responsible for serializing traces and cycling the output files so that no individual file becomes to large. - A buffer for aggregating traces to allow for batched flushes. - An agent which initializes the tracing controller and ensures that trace serialization is done on a separate thread. - A set of macros for generating trace events. - Tests and documentation. Author: Raymond Kang <[email protected]> Author: Kelvin Jin <[email protected]> Author: Matthew Loring <[email protected]> Author: Jason Ginchereau <[email protected]> PR-URL: #9304 Backport PR-URL: #11106 Reviewed-By: Josh Gavant <[email protected]>
joshgav commented Feb 28, 2017
Rebased cleanly (and force-pushed to @matthewloring's branch). Still need another LGTM I think? @nodejs/diagnostics PTAL. |
matthewloring commented Feb 28, 2017
LGTM but I'm not sure I count since I opened the PR. |
This commit adds support for trace-event tracing to Node.js. It provides a mechanism to centralize tracing information generated by V8, Node core, and userspace code. It includes: - A trace writer responsible for serializing traces and cycling the output files so that no individual file becomes to large. - A buffer for aggregating traces to allow for batched flushes. - An agent which initializes the tracing controller and ensures that trace serialization is done on a separate thread. - A set of macros for generating trace events. - Tests and documentation. Author: Raymond Kang <[email protected]> Author: Kelvin Jin <[email protected]> Author: Matthew Loring <[email protected]> Author: Jason Ginchereau <[email protected]> PR-URL: nodejs#11106 Reviewed-By: Josh Gavant <[email protected]>
italoacasas commented Mar 1, 2017
Landed |
Trott commented Mar 3, 2017
Adding |
jasnell commented Mar 3, 2017
selected semver-minor's can land on LTS branches. It people feel that it is worthwhile, then mention @nodejs/lts in the comments and a decision can be made. |
matthewloring commented Mar 3, 2017
This change is not compatible with the versions of V8 in 4.x and 6.x. |
This commit adds support for trace-event tracing to Node.js. It provides
a mechanism to centralize tracing information generated by V8, Node
core, and userspace code. It includes:
output files so that no individual file becomes to large.
trace serialization is done on a separate thread.
Author: Raymond Kang [email protected]
Author: Kelvin Jin [email protected]
Author: Matthew Loring [email protected]
Author: Jason Ginchereau [email protected]
PR-URL: #9304
Reviewed-By: Trevor Norris [email protected]
Reviewed-By: Michael Dawson [email protected]
Reviewed-By: Josh Gavant [email protected]