Skip to content
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

doc: meeting minutes 2017-02-23 #88

Merged
merged 1 commit into from
Mar 24, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
266 changes: 266 additions & 0 deletions wg-meetings/2017-02-23.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
# Diag WG Meeting - February 2017

* Date: 2017-02-23
* No recording due to technical difficulties.
* Minutes:
<https://docs.google.com/document/d/1Rt6yFAgCSmBFYpWDd9JL90f8BWG-AhhiJ7gOluRo-XQ>
* Previous meeting minutes:
<https://github.com/nodejs/diagnostics/blob/master/wg-meetings/2017-01-19.md>

## Attendees

* Josh Gavant @joshgav
* Ali Ijaz Sheikh @ofrobots
* Matthew Loring @matthewloring
* Andreas Madsen @andreasmadsen
* Richard Lau @richardlau
* Richard Chamberlain @rnchamberlain
* Eugene Ostroukhov @eugeneo
* Zbigniew Tenerowicz @naugtur

---

## Previous Meeting Review

Notes: <https://github.com/nodejs/diagnostics/blob/master/wg-meetings/2017-01-19.md>

### Inspector

* Support for HTTP/WS inspection
[diagnostics#75](https://github.com/nodejs/diagnostics/issues/75)
* Decide if/how to add trace listeners to Trace Controller.

* src, lib: deprecate old debug-agent and CLI debugger
[node#10276](https://github.com/nodejs/node/pull/10276)
* @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](https://github.com/nodejs/node/pull/8978)
* @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.

* Bringing node-inspect into the fold
[diagnostics#67](https://github.com/nodejs/diagnostics/issues/67)
* 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](https://github.com/nodejs/post-mortem/issues/40)
* Continue discussion in GH.

### Trace

* Trace Events trace system in Node.js
[diagnostics#53](https://github.com/nodejs/diagnostics/issues/53)
* trace_event.h tracking issue
[diagnostics#30](https://github.com/nodejs/diagnostics/issues/30)
* proposal: JS tracing APIs
[node-eps#48](https://github.com/nodejs/node-eps/pull/48)
* @joshgav to close existing issues and open new ones to track ongoing work
* Read doc and provide feedback to Jason.

### Async Context

* async_hooks initial implementation
[node#8531](https://github.com/nodejs/node/pull/8531)
* What will Domain be replaced with?
[node#10843](https://github.com/nodejs/node/issues/10843)

### Post-Mortem

* [node-report](https://github.com/nodejs/nodereport)
* [llnode](https://github.com/nodejs/llnode)

---

## Today’s Agenda

* proposal: programmatically expose the V8 inspector URL
[node#11496](https://github.com/nodejs/node/issues/11496)
* inspector: make `debug` an alias for `inspect`
[node#11441](https://github.com/nodejs/node/pull/11441)
* Switch the CLI debugger to V8 inspector
[node#11421](https://github.com/nodejs/node/issues/11421)
* \[WIP\] inspector: hint text update
[node#11207](https://github.com/nodejs/node/pull/11207)
* What will Domain be replaced with?
[node#10843](https://github.com/nodejs/node/issues/10843)
* src: separate trace_event_common from trace_event
[node#10628](https://github.com/nodejs/node/pull/10628)
* async_hooks initial implementation
[node#8531](https://github.com/nodejs/node/pull/8531)

* wg: nominating jkrems
[diagnostics#87](https://github.com/nodejs/diagnostics/pull/87)
* Request to join Diagnostics WG
[diagnostics#86](https://github.com/nodejs/diagnostics/pull/86)
* \[trace\] tracking issue; out of experimental
[diagnostics#84](https://github.com/nodejs/diagnostics/issues/84)
* [async_hooks] tracking issue
[diagnostics#29](https://github.com/nodejs/diagnostics/issues/29)

* guides: debugging getting started guide
[nodejs.org#1131](https://github.com/nodejs/nodejs.org/pull/1131)
* blog: diag wg update and --debug deprecation
[nodejs.org#1156](https://github.com/nodejs/nodejs.org/pull/1156)

---

## Minutes

### proposal: programmatically expose the V8 inspector URL [#11496](https://github.com/nodejs/node/issues/11496)

How should we properly expose metadata? @eugeneo working on an in-process JS API
to get this.

UUID would be valuable for other bindings and transports too.

**Next steps**

* Address via @eugeneo’s JS API work.

---

### inspector: make `debug` an alias for `inspect` [#11441](https://github.com/nodejs/node/pull/11441)

Best to focus on Inspector in Node 8+ rather than continue with 2 mechanisms. We
should at least drop support for `node debug` with the current agent
(`./lib/_debug_agent.js`).

Should we consider dropping `node debug` entirely? Policy requires longer period
deprecation.

If so, should we make `--debug` an alias to `--inspect` too? Yes, but only in
8.x. That’s included in this PR. In 7.x we should only print the deprecation
warning.

---

### Switch the CLI debugger to V8 inspector [#11421](https://github.com/nodejs/node/issues/11421)

Is bringing Inspector out of experimental dependent on items in this issue?
@ofrobots will suggest an appropriate subset.

This is a tracking issue so we’ll keep it tagged `diag-agenda`.

---

### [WIP] inspector: hint text update [#11207](https://github.com/nodejs/node/pull/11207)

Proposal: IP, port, and UUID, followed by line pointing to debugging guide.
Don’t land till we’re out of experimental.

Chrome metrics seem to suggest lots of people coming in via copy-pasting the
URL. Might want to make sure usability is not negatively affected.

Webstorm, VS Code don't typically need a URL because they start the process.
Perhaps it makes sense to have the URL for Chrome only, but not the other?

**Next steps**

* Josh to update PR, continue discussion there.

---

### src: separate trace_event_common from trace_event [#10628](https://github.com/nodejs/node/pull/10628)

How should Node relate to upstream trace_event_common?

Work in progress on moving trace APIs to a libtracing library usable by Node,
Chromium, etc. We’ll be able to depend on that upstream, so don’t need to worry
too much about current source arrangement, and fine to refactor as needed in
Node in the meantime.

Should we hold on further Trace work till libtracing is ready? No, the 4 macros
suggested for use in Node will still be the same.

**Next steps**

* Continue discussion in issue, remove from diag-agenda.

---

### wg: nominating jkrems [#87](https://github.com/nodejs/diagnostics/pull/87)

No objections, landed.

### Request to join Diagnostics WG [#86](https://github.com/nodejs/diagnostics/pull/86)

No objections, already merged.

Is current policy requiring majority approval appropriate? @ofrobots: It should
be less strict. @joshgav: Some permissions tied to membership like access to CI
jobs, so basic approval needed.

Group membership is quite large and many members don’t participate, which
hampers ability to get quorums, majorities, etc.

@joshgav: TSC is working on basic requirements for membership in
[TSC#142](https://github.com/nodejs/TSC/pull/142). Once that lands we can apply
principles to Diag WG too.

Suggestion: Ask current members if they want to continue membership.

**Next steps**

* Josh to reach out to current members.

---

### [trace] tracking issue [#84](https://github.com/nodejs/diagnostics/issues/84)

This is a tracking issue so we’ll keep it tagged `diag-agenda`.

---

### [async_hooks] tracking issue [#29](https://github.com/nodejs/diagnostics/issues/29)

* Related
* What will Domain be replaced with?
[#10843](https://github.com/nodejs/node/issues/10843)
* async_hooks initial implementation
[#8531](https://github.com/nodejs/node/pull/8531)

To be able to implement zones or domains or other solutions on top of
async_hooks, needed support for promise hooks. That’s available in V8 5.7, so
ready for someone to integrate that.

Asked Trevor for an update in issue. Note that he recently had a baby.

This is a tracking issue so we’ll keep it tagged `diag-agenda`.

---

### guides: debugging getting started guide [#1131](https://github.com/nodejs/nodejs.org/pull/1131)

Request for review and +1 or comments.

### blog: diag wg update and --debug deprecation [nodejs.org#1156](https://github.com/nodejs/nodejs.org/pull/1156)

Request for review and +1 or comments.

---

### node-report overview (@rnchamberlain)

node-report provides some post-mortem info on signals and would be nice to
expand further. Need more hooks in Node and V8 to get more info.

Should node-report go into core (nodejs/node)? Initially some discussion on this
in nodejs/node core, but sentiment was to keep core small.

@ofrobots: Would be useful to integrate in core. Anything not accessible through
JavaScript should be part of core. Not all users can use native modules.

Could we include it in `./deps` like node-inspect and integrate from there into
core? Might be better to put it in libuv or V8.

**Next steps**

* @rnchamberlain to open issue regarding integration in core.