-
Notifications
You must be signed in to change notification settings - Fork 78
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
137 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
# Node.js Diagnostics WG Meeting | ||
|
||
- Date: 2016-07-13 | ||
- YouTube: http://www.youtube.com/watch?v=BtZ6aFrD8gQ | ||
|
||
## Attendees | ||
|
||
- Josh Gavant (@joshgav) | ||
- Michael Dawson (@mhdawson) | ||
- Thomas Watson (@watson) | ||
- Andreas Madsen (@andreasmadsen) | ||
- Matt Loring (@matthewloring) | ||
- Ali Sheikh (@ofrobots) | ||
- Raymond Kang (@misterpoe) | ||
- Cristian Cavalli (@cristiancavalli) | ||
- Daniel Khan (@danielkhan) | ||
- Trevor Norris (@trevnorris) | ||
|
||
## Agenda | ||
|
||
- AsyncWrap - [[nodejs/node-eps#18](https://github.com/nodejs/node-eps/pull/18)] | ||
- Trace Controller [[nodejs/diagnostics#53](https://github.com/nodejs/diagnostics/issues/53)] | ||
- Inspector Protocol [[nodejs/node#7393](https://github.com/nodejs/node/issues/7393)] | ||
- APM Summit [[nodejs/diagnostics#58](https://github.com/nodejs/diagnostics/issues/58) | ||
|
||
## Minutes | ||
|
||
## AsyncWrap | ||
|
||
- [[nodejs/node-eps#18](https://github.com/nodejs/node-eps/pull/18)] | ||
- [[nodejs/diagnostics#29](https://github.com/nodejs/diagnostics/issues/29)] | ||
|
||
@trevnorris: Do not have ability to instrument promises and it doesn’t look promising. | ||
|
||
Follow [[nodejs/diagnostics#29](https://github.com/nodejs/diagnostics/issues/29)], create separate issues for specific items. | ||
|
||
**Questions**: | ||
|
||
- Do we need to deprecate `AsyncWrap::MakeCallback`? @trevnorris: eventually. | ||
- What about the duplication of `node::MakeCallback` and `AsyncWrap::MakeCallback`? @trevnorris: There will be less duplication but some is still needed. | ||
- User modules cannot currently be instrumented by AsyncWrap. | ||
- How will AsyncWrap be integrated in NaN's callback wrapper? | ||
- How will AsyncWrap be implemented in the in-progress ABI-stable API? | ||
|
||
**Next steps**: | ||
|
||
- One significant change remains to enable multiple listeners. | ||
- @trevnorris is working on a JS wrapper module for AsyncWrap to make it more usable for end users. | ||
|
||
|
||
## Tracing Controller | ||
|
||
- [[nodejs/diagnostics#53](https://github.com/nodejs/diagnostics/issues/53)] | ||
|
||
Some components will live in V8, some in Node. PR with V8 component: <https://codereview.chromium.org/2137013006/> | ||
|
||
Will provide interface for configuring trace collection and serialization format. Will also provide aggregation and filtering capabilities. | ||
|
||
Prototyped a Node-specific subclass of V8 component and able to get V8 traces. Added some trace points in `fs` to test. | ||
|
||
@joshgav: Will there be an interface for user modules (native or JS) to read and write traces? | ||
|
||
@ofrobots: Goal is to get basics in, then it’s up to Node.js to decide if/how to expose to users. | ||
|
||
@mattloring: Currently based on C++ macros and included directly in code. | ||
|
||
**Q**: What information is provided in a trace? | ||
|
||
**A**: CPU, TID, PID, timestamp. Follow-on work planned to allow alternate arguments. Also, some TRACE_EVENT macros accept additional metadata. | ||
|
||
**Q**: Where is output written to? | ||
|
||
**A**: Will provide a `TraceWriter` interface which users can derive from and write to file, stream, whatever. | ||
|
||
**Timeline**: V8 v5.4. | ||
|
||
**Next steps**: | ||
|
||
- Matt and Ali continue working on V8/Chromium implementation. | ||
- Add trace points throughout Node core codebase. | ||
- Define and implement interfaces for JS and native modules to read from and write to trace system. | ||
|
||
|
||
## Inspector | ||
|
||
[[nodejs/node#7393](https://github.com/nodejs/node/issues/7393)] | ||
|
||
@joshgav: Intent is to enable other engines to use same debugging protocol as V8 without having to implement V8's diagnostic APIs. | ||
|
||
Also need to update CLI debugger. | ||
|
||
Check `v8_inspector` tag in nodejs/node for needed contributions. | ||
|
||
|
||
## APM Summit | ||
|
||
[[nodejs/diagnostics#58](https://github.com/nodejs/diagnostics/issues/58)] | ||
|
||
|
||
@watson: At NodeConf Adventure discussed how APM providers are solving the same problem over and over again. Idea is a shared service in a module or part of core. | ||
|
||
Agenda would be: | ||
|
||
- Application-level traces especially for HTTP, DB requests (I/O). Currently requires ugly monkey-patching. AsyncWrap addresses some needs but not all. Key need is to keep a context across async boundaries. | ||
|
||
- Problem with ES modules because they can’t be monkey-patched. | ||
|
||
@joshgav: Can application-level traces be provided through the Tracing Controller? | ||
|
||
@ofrobots: TRACE_EVENT could be a source of information, but true need is to track async context. | ||
|
||
@watson: Can Trace Controller be used in production? | ||
|
||
@ofrobots: Yes, it’s designed for investigating perf issues, but it depends on number of events. | ||
|
||
@ofrobots: Think of Tracing Controller as “structural profiling” as opposed to "sampling profiling." | ||
|
||
Right now no user modules have been instrumented with TRACE_EVENT anyway. But eventually modules could be instrumented with same. | ||
|
||
@watson: Provide a standard API/format for tracing so all module authors could instrument their own code. | ||
|
||
@andreasmadsen: In what way does AsyncWrap not address this problem? | ||
|
||
@watson: example is postgres module, which maintains a pool of callbacks. Callbacks retain context in which pool was originally created [so AsyncWrap fails]. | ||
|
||
@ofrobots: AsyncWrap helps with Node core, but the notion of a logical context doesn’t exist in JS. | ||
|
||
Zones proposal was also an attempt to solve this problem. | ||
|
||
**Next steps**: | ||
|
||
- Many won’t be at Node Summit, we’ll postpone to a later conference. @watson will create a Doodle. | ||
|
||
|
||
## Next Meeting | ||
|
||
- Early September. |