src: allow setting a dir for all diagnostic output by AshCripps · Pull Request #33584 · nodejs/node

AshCripps · 2020-05-27T13:58:33Z

Add a flag that allows for the setting of a directory where all
diagnostic output will be written to.
e.g. --redirect-warnings

refs: #33010 (comment)

f.y.i @sam-github

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

doc/api/cli.md

sam-github

I like the idea, I think it helps, a couple suggestions for consistency.

doc/api/cli.md

src/node_options.cc

src/node_options.h

richardlau · 2020-05-29T15:24:01Z

This could use some tests, particularly that the new option is only changing the default (i.e. if a more specific option is also used then the value of that is used instead of the more generic one).

doc/api/cli.md

doc/node.1

AshCripps · 2020-06-02T12:18:38Z

Ive added some testing for the --cpu-prof-dir option with use of diagnostic-dir (I adapted https://github.com/nodejs/node/blob/master/test/sequential/test-cpu-prof-dir-absolute.js) but before I write any more I would appreciate some feedback if these are correct as ive never written node test before so not 100% sure if this is the correct way.

nodejs-github-bot · 2020-06-02T12:19:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31699/

joyeecheung · 2020-06-02T14:55:34Z

doc/api/cli.md

Considering these two dir flags are still experimental, I think we could just emit a deprecation warning for them, and make them no-ops when --diagnostic-dir is set (IIUC that's what this PR effectively does). Eventually we can just remove them.

um, on the other hand, maybe we would also want to allow the user to override these directories individually through --cpu-prof-dir and --heap-prof-dir even when --diagnostic-dir is set?

thats what I intented with:

if (cpu_prof && cpu_prof_dir.empty() && !diagnostic_dir.empty()) { cpu_prof_dir = diagnostic_dir; }

nodejs-github-bot · 2020-06-08T14:13:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31769/

nodejs-github-bot · 2020-06-08T16:05:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31773/

nodejs-github-bot · 2020-06-15T14:18:58Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31868/

nodejs-github-bot · 2020-06-16T09:56:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/31886/

nodejs-github-bot · 2020-07-14T15:18:43Z

CI: https://ci.nodejs.org/job/node-test-pull-request/32335/

nodejs-github-bot · 2020-07-15T22:24:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/32362/

Add a flag that allows for the setting of a directory where all diagnostic output will be written to. e.g. --redirect-warnings Refs: #33010 (comment) PR-URL: #33584 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax · 2020-07-20T16:20:42Z

Landed in 242bfb6

Add a flag that allows for the setting of a directory where all diagnostic output will be written to. e.g. --redirect-warnings Refs: #33010 (comment) PR-URL: #33584 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Notable changes: dgram: * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500 doc: * add AshCripps to collaborators (AshCripps) #34494 * add HarshithaKP to collaborators (Harshitha K P) #34417 * add rexagod to collaborators (Pranshu Srivastava) #34457 * add release key for Richard Lau (Richard Lau) #34397 events: * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057 src: * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387 * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584 worker: * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057 PR-URL: TODO

Notable changes: dgram: * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500 doc: * add AshCripps to collaborators (AshCripps) #34494 * add HarshithaKP to collaborators (Harshitha K P) #34417 * add rexagod to collaborators (Pranshu Srivastava) #34457 * add release key for Richard Lau (Richard Lau) #34397 events: * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057 src: * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387 * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584 worker: * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057 zlib: * switch to lazy init for zlib streams (Andrey Pechkurov) #34048 PR-URL: #34542

Notable changes: deps: * upgrade npm to 6.14.7 (claudiahdz) #34468 dgram: * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500 doc: * add AshCripps to collaborators (AshCripps) #34494 * add HarshithaKP to collaborators (Harshitha K P) #34417 * add rexagod to collaborators (Pranshu Srivastava) #34457 * add release key for Richard Lau (Richard Lau) #34397 events: * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057 src: * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387 * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584 worker: * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057 zlib: * switch to lazy init for zlib streams (Andrey Pechkurov) #34048 PR-URL: #34542

Add a flag that allows for the setting of a directory where all diagnostic output will be written to. e.g. --redirect-warnings Refs: #33010 (comment) PR-URL: #33584 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. labels May 27, 2020

jasnell reviewed May 27, 2020

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

jasnell reviewed May 27, 2020

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

sam-github suggested changes May 27, 2020

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

doc/api/cli.md Outdated Show resolved Hide resolved

src/node_options.cc Outdated Show resolved Hide resolved

src/node_options.h Outdated Show resolved Hide resolved

AshCripps force-pushed the add-diagnostic-output branch from 12a6f18 to 3606f54 Compare May 28, 2020 11:43

jasnell approved these changes May 29, 2020

View reviewed changes

AshCripps force-pushed the add-diagnostic-output branch from 7c7f2af to 9eb7c71 Compare May 29, 2020 11:57

AshCripps changed the title ~~[WIP] src: allow setting a dir for all diagnostic output~~ src: allow setting a dir for all diagnostic output May 29, 2020

AshCripps marked this pull request as ready for review May 29, 2020 12:03

sam-github suggested changes May 29, 2020

View reviewed changes

AshCripps force-pushed the add-diagnostic-output branch 2 times, most recently from 88645e3 to 10b123b Compare May 29, 2020 17:04

BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:18

AshCripps force-pushed the add-diagnostic-output branch from 10b123b to 00e69b6 Compare June 2, 2020 11:25

joyeecheung reviewed Jun 2, 2020

View reviewed changes

sam-github approved these changes Jun 2, 2020

View reviewed changes

AshCripps force-pushed the add-diagnostic-output branch from 0c0dd7c to f299035 Compare June 8, 2020 12:22

richardlau approved these changes Jun 8, 2020

View reviewed changes

This was referenced Jun 9, 2020

CI Relability 2020-06-09 JaneaSystems/nodejs-reliability#9

Open

CI Relability 2020-06-10 JaneaSystems/nodejs-reliability#10

Open

github-actions bot mentioned this pull request Jun 18, 2020

CI Relability 2020-06-18 JaneaSystems/nodejs-reliability#11

Open

18 tasks

src: correct diagnostic-dir doc links

900e397

AshCripps force-pushed the add-diagnostic-output branch from 7e323a6 to 900e397 Compare July 14, 2020 15:14

addaleax approved these changes Jul 14, 2020

View reviewed changes

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 14, 2020

addaleax added cli Issues and PRs related to the Node.js command line interface. semver-minor PRs that contain new features and should be released in the next minor version. and removed process Issues and PRs related to the process subsystem. labels Jul 15, 2020

addaleax closed this Jul 20, 2020

ruyadorno mentioned this pull request Jul 28, 2020

v14.7.0 proposal #34542

Merged

codebytere mentioned this pull request Sep 28, 2020

v12.19.0 proposal #35401

Merged

mhdawson mentioned this pull request Jan 18, 2022

pummel/test-worker-take-heapsnapshot - crash in debug mode on arm #41204

Closed

Uh oh!

Conversation

AshCripps commented May 27, 2020

Checklist

Uh oh!

Uh oh!

Uh oh!

sam-github left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

richardlau commented May 29, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AshCripps commented Jun 2, 2020

Uh oh!

nodejs-github-bot commented Jun 2, 2020

Uh oh!

joyeecheung Jun 2, 2020

Choose a reason for hiding this comment

Uh oh!

joyeecheung Jun 2, 2020

Choose a reason for hiding this comment

Uh oh!

AshCripps Jun 2, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 8, 2020

Uh oh!

nodejs-github-bot commented Jun 8, 2020

Uh oh!

nodejs-github-bot commented Jun 15, 2020

Uh oh!

nodejs-github-bot commented Jun 16, 2020

Uh oh!

nodejs-github-bot commented Jul 14, 2020

Uh oh!

nodejs-github-bot commented Jul 15, 2020

Uh oh!

addaleax commented Jul 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants