Uh oh!
There was an error while loading. Please reload this page.
src: trace_events: support for metadata events - #20757
Conversation
jasnell
commented
May 16, 2018
Oh, awesome! You just crossed one of my to-do items off! |
mcollina
left a comment
There was a problem hiding this comment.
LGTM.
Do we need to document those somewhere?
jasnell
commented
May 16, 2018
@mcollina ... I'll be doing a complete rework of the |
There was a problem hiding this comment.
Given that trace events can be enabled after this point, perhaps we should move this to ensure that it's always emitted when tracing is first turned on?
There was a problem hiding this comment.
fwiw... I plan on doing to same to emit some information about which node.js version is being traced
There was a problem hiding this comment.
Given that trace events can be enabled after this point, perhaps we should move this to ensure that it's always emitted when tracing is first turned on?
I think the right way to go here would be to address the TODO I already added to the code regarding metadata events being kept in a separate buffer of their own and being emitted on each flush of the trace buffer. That would address the case you mention (tracing turned on later) and more (streaming). I think the emit of the event should should stay where it logically make sense. For example, when we create a thread, we should emit the event at that location rather than artificially moving the emit to a point to better align with the lifecycle of tracing.
I plan to implement this secondary buffer for metadata events in a follow-on. I think it will address the issue.
fwiw... I plan on doing to same to emit some information about which node.js version is being traced
👍
ofrobots
commented
May 17, 2018
New CI after rebase: https://ci.nodejs.org/job/node-test-pull-request/14937/ |
Running Windows again: https://ci.nodejs.org/job/node-test-commit-windows-fanned/18094/ (All other failures are related due to a bug in our http2 code). |
Add support for metadata events. At this point they are added to the main buffer. Emit a metadata event for the main thread.
ofrobots
commented
May 18, 2018
Thanks. Landed in 3ff723f. |
Add support for metadata events. At this point they are added to the main buffer. Emit a metadata event for the main thread. PR-URL: #20757 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Add support for metadata events. At this point they are added to the main buffer. Emit a metadata event for the main thread. PR-URL: #20757 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Add support for metadata events. At this point they are added to the
main buffer. Emit a metadata event for the main thread.
Metadata events are implicitly enabled when tracing is enabled. They contain information pertaining not to the individual events as they occur but rather about the process and the environment. Trace viewers use this information, e.g. to render thread names.
Example:
Before
After:
/cc @nodejs/trace-events @nodejs/diagnostics
CI: https://ci.nodejs.org/job/node-test-pull-request/14896/