Skip to content

Latest commit

 

History

History
340 lines (224 loc) · 10 KB

2017-01-19.md

File metadata and controls

340 lines (224 loc) · 10 KB
Raw

Diag WG Meeting - January 2017

Attendees

  • Josh Gavant @joshgav
  • Matt Loring @matthewloring
  • Ali Sheikh @ofrobots
  • Mark Marron @mrkmarron
  • Jason Ginchereau @jasongin
  • Jeremiah Senkpiel @Fishrock123
  • Richard Chamberlain @rnchamberlain
  • Richard Lau @richardlau

Agenda

Inspector

Trace

Async Context

Post-Mortem

Previous Meeting

Trace

  • Intended use cases (profiling, logging, etc.)
  • JS API
  • Conventions for category names and payload fields
  • Trace points in core

Next steps:

  • 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 (listener) mode.

Inspector

  • Deprecate node --debug (old Debugger interface)
  • Deprecate CLI debugger (node debug)
  • Replace CLI debugger with node-inspect
  • Activate inspector in running process with SIGUSR1

Next steps:

  • In issue for removal of old debugger node#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 distribution.
  • Reply in node#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 Context

Next steps:

  • Review Embedder API added to async_hooks and start explaining to ecosystem.

Minutes

Inspector

RFC: Support for subprocess inspection diagnostics#77

Consensus that this is a good idea, move forward with an initial implementation.

Support for HTTP/WS inspection diagnostics#75

  • Should we support the Network domain in Inspector?
  • Should it go through trace_events?

@Fishrock123: Chrome DevTools UI doesn’t provide a good interface for Node’s traffic load.

@joshgav: Consider from protocol perspective independent of presentation.

@Fishrock123: Network connections are already (able to be) wrapped by async_hooks, why an additional sink?

@ofrobots: Capture network events through trace_event first. That capture could be built on top of async_hooks.

@Fishrock123: For example, a native module which implements async_hooks for network events and sends events to trace system.

Do we want to also expose to Network domain in Inspector? To add to Inspector protocol would need trace listeners for TraceController, something @JasonGin has raised.

@ofrobots to discuss with @eugeneo.

Next steps:

  • Decide if/how to add trace listeners to Trace Controller.
  • Continue discussion in GH.

src, lib: deprecate old debug-agent and CLI debugger node#10276

Josh proposes to separate deprecations into 2, land debug-agent deprecation in 7.x; CLI deprecation when node-inspect is integrated.

Will old debugger be gone in Node@8? Depends on whether we go with V8 5.7 or 5.8. 5.8 doesn’t include old debugger. It also includes Turbofan and removes --expose-debug-as flag.

The Diagnostics WG suggests that Node@8 use V8 5.7 so old debug interface remains available.

In that case we shouldn't land runtime deprecation in a 7.x release because a runtime deprecation is semver-major and we can land it in 8.x.

Get confirmation from CTC that V8 5.7 will be target for Node@8, then we can delay landing deprecation till 8.x.

@jkrems (comment): Isn't our policy to deprecate for two major versions?

Next steps

  • @joshgav to update runtime deprecation PR.
  • Ask CTC to advise on target V8 version for Node@8.
  • Continue discussion on whether to land a runtime deprecation in a 7.x release.

inspector, doc: update hint text, add guide node#8978

@joshgav to move guide to nodejs.org following guides reorganization.

  • Should we update the hint text? What should it be?
  • How do we move out of experimental state? Do we have to move out before 8.x?

Since we’re deprecating the old stuff in 8, seems new stuff should be out of experimental.

Need CTC vote/review to move Inspector out of experimental.

@Fishrock123: Open an issue/PR tagged ctc-agenda to bring out of experimental. Can be brought out in a minor.

CTC will want to verify that API (e.g. protocol, command-line flags) won’t change radically in near future. It would be okay to add APIs.

Current API: https://chromedevtools.github.io/debugger-protocol-viewer/v8/

Josh to ask ChakraCore about their needs and what it might require in current implementation.

Hint text:

  • Should include link to docs.
  • As long as experimental note that.
  • Would be nice to include links for use with CDT and others.

Next steps

  • @joshgav to move guide to nodejs.org following guides reorganization.
  • @ofrobots to ask for CTC review to move out of experimental state preferably in 8.x.
  • Josh to ask ChakraCore to review.
  • Continue discussion on hint text in GH.

node-inspect

Do we want the node inspect script.js and/or node debug script.js experience?

Yes, good to have basic debugging built in a la node debug.

Approved by TSC and CTC to bring into Node.js Foundation, @joshgav to work with org owners and @jkrems to migrate.

Move bnoordhuis/node-heapdump to nodejs/node-heapdump post-mortem#40

@joshgav: Suggest integrating this functionality into node-inspect since Inspector provides this functionality.

@ofrobots: node-inspect may not fully address the use case here so we should consider this independently.

@Fishrock123: should this be added to the V8 module in core?

@joshgav: Should we instead factor V8 module out to indicate support for other VMs?

@Fishrock123 - just a name, could rename.

Next steps:

  • Continue discussion in GH.

Trace

@joshgav to close existing issues and open new ones to track ongoing work on the following:

  • add listen capability
  • add JS API
  • add trace points to core
  • add C++ API over macros?
  • conventions for category names, payloads
  • what is needed to land in a release, and as non-experimental?

proposal: JS tracing APIs node-eps#48

Purpose is not just for async, but for any perf diagnostics - counters, instant operations, begin/end for synchronous operations, etc.

Allows JS module authors to expose tracing data from their modules.

async_hooks is only for tracking async context.

In EP, seeking feedback on what should go in core or not?

Next steps

  • Read doc and provide feedback to Jason.

Async Context

  • async_hooks initial implementation node#8531

  • What will Domain be replaced with? node#10843

  • docs and tests for async_hooks to be added to PR soon

@AndreasMadsen (comment):

A while ago Trevor said the following were missing:

  • Remove all weak objects (will be handled in another PR to solve another issue)
  • Add support to node::MakeCallback
  • Implement public native API.
  • Detect a change in the hooks queue when in the middle of running hooks, so the original set of hooks can finish executing.

My best guess is that only "Add support to node::MakeCallback" and "Implement public native API" is missing now, but that he wants to test the current implementation first.

Post-Mortem

Human-readable “dump.”

MDB story for core dumps on Linux and Mac.

Next

  • Create a label for WG agenda.
  • Create a Doodle for next meeting in a few weeks.