src: add --title command line argument, trace event #21477

jasnell · 2018-06-22T23:27:16Z

Adds a simple utility --title command line argument to set the process title on startup. Also, emit the process_name metadata to the trace event log.

$ node --title=foo -pe "process.title"
foo

When emit trace events, the process name appears within the trace event viewer UI:

$ node --title=foo --trace-event-categories node -pe "process.title"
foo

Changes to process.title also appear within the trace_event log:

/cc @ofrobots @eugeneo

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

The text was updated successfully, but these errors were encountered:

nodejs-github-bot · 2018-06-22T23:27:17Z

@jasnell build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/128/pipeline

jasnell · 2018-06-22T23:27:52Z

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

jasnell · 2018-06-23T00:30:02Z

some relevant failures in CI.

jasnell · 2018-06-23T01:10:53Z

@ofrobots ... when you get a moment, can you take a look at this? It would appear that TRACE_STR_COPY() may not be actually be copying for metadata events, but it's difficult to verify.

vsemozhetbyt · 2018-06-23T04:38:39Z

doc/node.1

@@ -164,6 +164,9 @@ instead of printing to stderr.
 .It Fl -throw-deprecation
 Throw errors for deprecations.
 .
+.It Fl -title Ns = Ns Ar title
+Specify the process.title on startup


Nit: missing period.

Nit: remove the

Trott · 2018-06-23T05:03:18Z

doc/api/cli.md

+added: REPLACEME
+-->
+
+Set the `process.title` on startup.


Nit: remove the

jasnell · 2018-06-24T22:15:21Z

Still exploring why the trace event bit is failing on some of the CI machines.. unable to recreate locally... https://ci.nodejs.org/job/node-test-pull-request/15587/

addaleax · 2018-06-25T13:26:54Z

src/node.cc

@@ -2813,6 +2817,8 @@ static void ParseArgs(int* argc,
    } else if (strncmp(arg, "--security-revert=", 18) == 0) {
      const char* cve = arg + 18;
      Revert(cve);
+    } else if (strncmp(arg, "--title=", 8) == 0) {
+      uv_set_process_title(arg + 8);


Are you sure this is safe? Setting the process title (which on a lot of OS works by overwriting argv) while iterating over argv?

Possibly not ;) still verifying and chasing down a few issues that are likely being caused by exactly this problem. I have a change staged locally that moves the actual set_process_title call to after the args are processed but haven't pushed it up to test yet

Is the addition of --title tied to the trace event things, or is it a convenience thing? It seems like it’s pretty easy already to set the process title from inside the JS code, so it might be okay to leave this out…?

They are not tied together but the use case is closely related for me. The introduction of --title is specifically for cases of profiling other users code that I don't exactly want to have to modify just to set the title... using a preload module is a bit overkill for that.

jasnell · 2018-06-25T18:32:17Z

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

jasnell · 2018-06-25T20:55:18Z

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

This is failing on SmartOS only at this point.

Simple utility command line argument for setting the process title on process startup.

jasnell · 2018-06-25T22:18:00Z

Ok, should be good to go now: https://ci.nodejs.org/job/node-test-pull-request/15615/

jasnell · 2018-06-25T23:53:01Z

CI is good. One unrelated flaky failure in windows.

Trott · 2018-06-26T05:33:14Z

CI is good. One unrelated flaky failure in windows.

Then it's not quite good yet. :-D

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

jasnell · 2018-06-29T21:10:26Z

Let's give it one more shot since the unrelated windows failure happened again https://ci.nodejs.org/job/node-test-pull-request/15676/

PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Simple utility command line argument for setting the process title on process startup. PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

jasnell · 2018-07-11T00:34:03Z

Landed in 6705356 and 9d71619

PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Simple utility command line argument for setting the process title on process startup. PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477)

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * inspector: * Expose the original console API in `require('inspector').console`. (#21659) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

nodejs-github-bot added the c++ label Jun 22, 2018

bmeck approved these changes Jun 22, 2018

View changes

jasnell added the wip label Jun 23, 2018

jasnell force-pushed the trace-event-data branch from 8b9d470 to 51111a8 Jun 23, 2018

vsemozhetbyt reviewed Jun 23, 2018

View changes

Trott reviewed Jun 23, 2018

View changes

doc/api/cli.md Outdated

added: REPLACEME

-->

Set the `process.title` on startup.

Copy link

Member

Trott Jun 23, 2018

Nit: remove the

Trott added the semver-minor label Jun 23, 2018

addaleax reviewed Jun 25, 2018

View changes

jasnell added 2 commits Jun 25, 2018

trace_events: add process_name metadata

62efea9

src: add --title command line argument

550335e

Simple utility command line argument for setting the process title on process startup.

jasnell force-pushed the trace-event-data branch from b8c686b to 550335e Jun 25, 2018

addaleax approved these changes Jun 25, 2018

View changes

addaleax removed the wip label Jun 25, 2018

Trott added the author ready label Jun 30, 2018

jasnell added a commit that referenced this issue Jul 11, 2018

trace_events: add process_name metadata

6705356

PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

jasnell closed this Jul 11, 2018

targos added a commit that referenced this issue Jul 12, 2018

trace_events: add process_name metadata

c4d7413

PR-URL: #21477 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

targos mentioned this pull request Jul 17, 2018

v10.7.0 proposal #21851

Merged

beevelop mentioned this pull request Jul 18, 2018

2018-07-18 Version 10.7.0 (Current) @targos beevelop/docker-nodejs#207

Closed

beevelop mentioned this pull request Jul 18, 2018

2018-07-18 Version 10.7.0 (Current) @targos beevelop/docker-nodejs#208

Closed

beevelop mentioned this pull request Jul 18, 2018

2018-07-18 Version 10.7.0 (Current) @targos beevelop/docker-nodejs#209

Closed

beevelop mentioned this pull request Jul 19, 2018

2018-07-18 Version 10.7.0 (Current) @targos beevelop/docker-nodejs#210

Closed

beevelop mentioned this pull request Jul 19, 2018

2018-07-18 Version 10.7.0 (Current) @targos beevelop/docker-nodejs#211

Closed

nodejs / node Public

src: add --title command line argument, trace event #21477

src: add --title command line argument, trace event #21477

jasnell commented Jun 22, 2018

nodejs-github-bot commented Jun 22, 2018

jasnell commented Jun 22, 2018

jasnell commented Jun 23, 2018

jasnell commented Jun 23, 2018 •

edited

vsemozhetbyt Jun 23, 2018

Trott Jun 23, 2018

Trott Jun 23, 2018

jasnell commented Jun 24, 2018

addaleax Jun 25, 2018

jasnell Jun 25, 2018

addaleax Jun 25, 2018

jasnell Jun 25, 2018 •

edited

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

Trott commented Jun 26, 2018

jasnell commented Jun 29, 2018

jasnell commented Jul 11, 2018

nodejs / node Public

src: add --title command line argument, trace event #21477

src: add --title command line argument, trace event #21477

Conversation

jasnell commented Jun 22, 2018

Checklist

nodejs-github-bot commented Jun 22, 2018

jasnell commented Jun 22, 2018

jasnell commented Jun 23, 2018

jasnell commented Jun 23, 2018 • edited

vsemozhetbyt Jun 23, 2018

Trott Jun 23, 2018

Trott Jun 23, 2018

jasnell commented Jun 24, 2018

addaleax Jun 25, 2018

jasnell Jun 25, 2018

addaleax Jun 25, 2018

jasnell Jun 25, 2018 • edited

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

jasnell commented Jun 25, 2018

Trott commented Jun 26, 2018

jasnell commented Jun 29, 2018

jasnell commented Jul 11, 2018

jasnell commented Jun 23, 2018 •

edited

jasnell Jun 25, 2018 •

edited