diff --git a/wg-meetings/2016-12-02.md b/wg-meetings/2016-12-02.md new file mode 100644 index 0000000..d76e888 --- /dev/null +++ b/wg-meetings/2016-12-02.md @@ -0,0 +1,238 @@ +# Diagnostics WG Meeting + +* Date: 2016-12-02 (Collaborators' Summit - Austin) +* GitHub issue: https://github.com/nodejs/summit/issues/30 +* Raw notes: + +## Agenda: + +* TracingController +* Inspector and Protocol +* async_hooks + +--- + +## TracingController + +Agenda: + +* Intended use cases +* JS API +* Trace points in Core + + +### Intended use cases + +For use with timing; intended first for statistical and event-based profiling. + +If people would use for broader event logging there could be perf implications. + +Could it be used as an alternative form for string-based logging (like +console.log) too, and address some of the problems with console.log? + +Core thesis is that profiling, tracing, etc. are all the same, all track +something with a timestamp. + +Can timestamp be correlated with other events? Within the same process, +`process.hrtime` also uses the same clock so can be used for correlation. + +What about a shared clock across processes? No, but TRACE_EVENT has a clock +sync event we might investigate. + + +### JS API + +Is there anything we’re risking, e.g. perf or security, by exposing to userland? + +* Maybe, but it’s worse with current implementations. +* If we allow arbitrary messages in a trace API, could spend too much time + building strings which aren’t used. Need to expose API in a way that only does + these steps when needed. + +Should we provide this API through `console`, e.g. `console.trace` or somesuch? + +Need a convention for category naming in userland. + +* use package names? + +Do we need to define a convention for the `args` structures? + +* For what? What beyond the core fields defined in [Chrome Tracing Format][] + might be broadly needed in userland modules? + +Do we need a "level"-like field as part of the core set? It would have +categories like Error, Warning, and Info and could be a primary field for filtering. + +* Could use a string in the category, e.g. `pkgname:verbose`. But will + authors use many different names, e.g. "verbose" and "info"? + +[Chrome Tracing Format]: https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/ + +Consensus is that we should expose this to userland. + + +### Trace Points in Core + +First step is to use macros throughout C++ in Node core and develop more opinions. + +Specifically: + +* libuv barriers, especially async_wrap +* to track event loop metrics +* current usages of debug.log + +Would be ideal to be able to choose categories with a regex. To do so need to add regex parsing to current impl. + + +### What remains to land PR? + +Provide an experimental warning if `--enable-tracing` runtime flag is specified. + +@mhdawson: Can this implementation be abstracted so as not to depend on V8? + +@mattloring: Work in progress to abstract this infrastructure to an independent +tracing library (libtracing). + +@mhdawson: Should we wait for that then? + +@ofrobots: To use that new implementation once available we'll only have to change +the build configuration. + +@mattloring: The actual interface is the macros, which will not change. + +Discussion to continue in GitHub +(https://github.com/nodejs/node/pull/9304#discussion_r90739052) + +* Wrap macros for simpler use? Not useful to add a layer in Node just for this. +* Specify log location? Could be added later. +* Programmatic API to access buffers? Could be added later. +* Is there a way to listen for events? Could be added later. + * Does Chromium have sinks we can reuse, e.g. for ETW. There may be one in Chromium to start from. +* Check SmartOS failure. + +Unless there’s objection, go ahead and land. + + +### **Next steps** for TracingController: + +* Add an "experimental" warning if `--enable-tracing` is specified. +* Discuss if/how to abstract from V8. +* If no objections, land initial PR (#9304). +* List next tasks in Diag repo, such as instrumenting the wraps in core, adding a JS + API, and adding a callback mode. + + +--- + +## Inspector and Protocol + +Old V8 debugger to be removed: https://github.com/nodejs/node/issues/9789. + +But our deprecation policy is typically at least 2 majors. @ofrobots to +investigate. + +What needs to be deprecated in Node as part of removal of old V8 debugger? + +* `lib/_debugger.js` +* use of special debug Context, e.g. as returned by [GetDebugContext](https://github.com/v8/v8/blob/02c6b04179a632a3d7a493e60086e5f4c5f47f95/include/v8-debug.h#L251). +* other uses of [include/v8-debug.h](https://github.com/v8/v8/blob/02c6b04179a632a3d7a493e60086e5f4c5f47f95/include/v8-debug.h)? + +**Next steps**: + +* In issue for removal of old debugger [node#9789](https://github.com/nodejs/node/issues/9789) discuss what else will be effected/deprecated. + +### CLI Debugger + +We’ll need to deprecate `node debug` soon since it’s based on deprecated protocol. + +Should we replace `lib/_debugger.js` (the CLI debugger, used through `node +debug`) with one that implements the new protocol? + +[node-inspect](https://github.com/nodejs/diagnostics/issues/67) by @jkrems can do +that. + +Questions to answer: + +* Does Node need an official CLI debugger, maintained in core? +* Should `node-inspect` be in the Foundation? +* Should it be bundled in default distribution? +* Should it be in `./deps` or `./lib`? + +What are the use cases which would justify it in core? When no GUI is +available. Still, could be acquired via a package manager. What about if no +network/package manager is available? + +Do other langs have built-in debuggers? Some. + +**Consensus** that node-inspect should be in the Foundation. + +Continue discussion on bundling in Core on GitHub. + +**Next steps**: + +* Open a specific issue for deprecating existing CLI debugger + (`lib/_debugger.js`, `node debug`). +* Open an issue in the TSC repo to bring node-inspect into the Foundation. +* Continue discussion on whether and how a CLI debugger should be bundled. + +### Activate inspector in running process + +[node#8464](https://github.com/nodejs/node/issues/8464) + +Final suggested options are: + +1. Activate both inspector and debugger and turn one off based on initial message. +2. Don’t do anything and switch default to activate inspector in 8.0. + +Consensus is option 2 and to switch the default for SIGUSR1 **now** in master only to +activate inspector. This would land in 8.x releases. + +**Next steps**: + +* Reply in existing issue that we plan to completely switch in v8.x. +* Notify client debugger projects. +* Message to community. + +### **Next steps** for Inspector and Protocol + +* In issue for removal of old debugger [node#9789](https://github.com/nodejs/node/issues/9789) discuss what else will be effected/deprecated. +* Open a specific issue for deprecating existing CLI debugger (`lib/_debugger.js`, `node debug`). +* Open an issue in the TSC repo to bring node-inspect into the Foundation. +* Continue discussion on whether and how a CLI debugger should be bundled in default distribition. +* Reply in [node#8464](https://github.com/nodejs/node/issues/8464) that we plan to completely switch to inspector in v8.x. +* Notify client debugger projects about switch of SIGUSR1. +* Message changes to community. + + +--- + +## async_hooks & context propagation + +* PR: https://github.com/nodejs/node/pull/8531 +* Node-EP: https://github.com/nodejs/node-eps/pull/18 + +How can we help the ecosystem with this problem of tracking context across async +boundaries? + +Domains and Zones were attempts. + +Could we add a context variable whenever a function is registered? Problem is +that it's not clear what context the programmer would intend to capture. + +async_hooks only works with Node’s native object hierarchy (\*Wraps). What about +callbacks handled outside of Node’s hierarchy or in userland? E.g. Bluebird. + +Userland queueing of callbacks breaks the async tracking mechanisms we’ve tried, +cause many callbacks come from a single source. + +async_hooks originally had this problem, trying to address by adding a userland +API - "Embedder API." This will allow module owners to fire their own hooks. + +Embedder API doesn’t help modules which are entirely in JS. + +Do we have a native (C++) API for async_hooks? Not at the moment, but could be added. + +**Next steps**: + +* Review Embedder API added to async_hooks and start explaining to ecosystem. +