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

sam-github

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

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).

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

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

Comment threaddoc/api/cli.md Outdated

jasnell reviewed May 27, 2020
View reviewed changes

Comment threaddoc/api/cli.md Outdated

sam-github suggested changes May 27, 2020
View reviewed changes

Comment threaddoc/api/cli.md Outdated
Comment threaddoc/api/cli.md Outdated
Comment threadsrc/node_options.cc Outdated
Comment threadsrc/node_options.h Outdated

AshCripps force-pushed the add-diagnostic-output branch from 12a6f18 to 3606f54Compare 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 9eb7c71Compare 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 outputMay 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

Comment threaddoc/api/cli.md Outdated
Comment threaddoc/api/cli.md Outdated
Comment threaddoc/api/cli.md Outdated
Comment threaddoc/api/cli.md Outdated
Comment threaddoc/api/cli.md Outdated
Comment threaddoc/node.1 Outdated

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

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

AshCripps force-pushed the add-diagnostic-output branch from 10b123b to 00e69b6Compare 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 f299035Compare 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-actionsBot 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 900e397Compare 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!

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!

joyeecheungJun 2, 2020

Choose a reason for hiding this comment

Uh oh!

joyeecheungJun 2, 2020

Choose a reason for hiding this comment

Uh oh!

AshCrippsJun 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