Skip to content

src: allow setting a dir for all diagnostic output #33584

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.

Sign up for GitHub

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

Closed
AshCripps wants to merge 4 commits into from
Closed

src: allow setting a dir for all diagnostic output #33584

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c763920
src: allow setting a dir for all diagnostic output
May 27, 2020
6d40876
src: add cpu-prof and heap-prof to diagnostic-dir
May 28, 2020
57e0028
src: add testing for --diagnostic-dir option
Jun 2, 2020
900e397
src: correct diagnostic-dir doc links
Jul 14, 2020
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
Prev Previous commit
Next Next commit
src: add cpu-prof and heap-prof to diagnostic-dir
  • Loading branch information
AshCripps authored and AshCripps committed Jun 19, 2020
commit 6d408769c423e3968387ce0680e3b27c18d5aef1
17 changes: 16 additions & 1 deletion doc/api/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ added: v12.0.0
Specify the directory where the CPU profiles generated by `--cpu-prof` will
be placed.

The default value is controlled by the
[--diagnostic-dir](#--diagnostic-dir=directory) command line option.
Comment thread
addaleax marked this conversation as resolved.
Outdated

### `--cpu-prof-interval`
<!-- YAML
added: v12.2.0
Expand All @@ -129,9 +132,14 @@ Specify the file name of the CPU profile generated by `--cpu-prof`.

### `--diagnostic-dir=directory`

Set the directory for which all diagnostic output files should be written to.
Set the directory to which all diagnostic output files will be written to.
Defaults to current working directory.

Affects the default output directory of:
* [--cpu-prof-dir](#--cpu-prof-dir)
Copy link
Copy Markdown
Member

@joyeecheung joyeecheung Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@joyeecheung joyeecheung Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

@AshCripps AshCripps Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats what I intented with:

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

* [--heap-prof-dir](#--heap-prof-dir)
* [--redirect-warnings](#--redirect-warnings=file)

### `--disable-proto=mode`
<!-- YAML
added:
Expand Down Expand Up @@ -365,6 +373,9 @@ added: v12.4.0
Specify the directory where the heap profiles generated by `--heap-prof` will
be placed.

The default value is controlled by the
[--diagnostic-dir](#--diagnostic-dir=directory) command line option.

### `--heap-prof-interval`
<!-- YAML
added: v12.4.0
Expand Down Expand Up @@ -645,6 +656,10 @@ file will be created if it does not exist, and will be appended to if it does.
If an error occurs while attempting to write the warning to the file, the
warning will be written to stderr instead.

The `file` name may be an absolute path. If it is not, the default directory it
will be written to is controlled by the
[--diagnostic-dir](#--diagnostic-dir=directory) command line option.

### `--report-compact`
<!-- YAML
added:
Expand Down
17 changes: 17 additions & 0 deletions doc/node.1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ with a generated file name.
The directory where the CPU profiles generated by
.Fl -cpu-prof
will be placed.
The default value is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -cpu-prof-interval
The sampling interval in microseconds for the CPU profiles generated by
Expand All @@ -103,6 +106,13 @@ File name of the V8 CPU profile generated with
.It Fl -diagnostic-dir
Set the directory for all diagnostic output files.
Comment thread
AshCripps marked this conversation as resolved.
Outdated
Default is current working directory.
Set the directory to which all diagnostic output files will be written to.
Defaults to current working directory.
.
Affects the default output directory of:
.Fl -cpu-prof-dir .
.Fl -heap-prof-dir .
.Fl -redirect-warnings .
.
.It Fl -disable-proto Ns = Ns Ar mode
Disable the `Object.prototype.__proto__` property. If
Expand Down Expand Up @@ -185,6 +195,9 @@ with a generated file name.
The directory where the heap profiles generated by
.Fl -heap-prof
will be placed.
The default value is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -heap-prof-interval
The average sampling interval in bytes for the heap profiles generated by
Expand Down Expand Up @@ -302,6 +315,10 @@ in a compact format, single-line JSON.
Location at which the
.Sy diagnostic report
will be generated.
The `file` name may be an absolute path. If it is not, the default directory it will
be written to is controlled by the
.Fl -diagnostic-dir .
command line option.
.
.It Fl -report-filename
Name of the file to which the
Expand Down
9 changes: 9 additions & 0 deletions src/node_options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
}
}

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

if (!heap_prof) {
if (!heap_prof_name.empty()) {
errors->push_back("--heap-prof-name must be used with --heap-prof");
Expand All @@ -159,6 +163,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
errors->push_back("--heap-prof-interval must be used with --heap-prof");
}
}

if (heap_prof && heap_prof_dir.empty() && !diagnostic_dir.empty()) {
heap_prof_dir = diagnostic_dir;
}

debug_options_.CheckOptions(errors);
#endif // HAVE_INSPECTOR
}
Expand Down