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

Give a name to a Worker (for debugging purpose) #41589

Closed
shayyzhakov opened this issue Jan 19, 2022 · 11 comments · Fixed by #46832
Closed

Give a name to a Worker (for debugging purpose) #41589

shayyzhakov opened this issue Jan 19, 2022 · 11 comments · Fixed by #46832
Labels
feature request Issues that request new features to be added to Node.js. worker Issues and PRs related to Worker support.

Comments

@shayyzhakov
Copy link

What is the problem this feature will solve?

While debugging my Node.js app in VS Code, I've been trying to figure out which of the workers that are running is responsible for what, but found it hard to follow the flow as the workers were named Worker ${index} instead of having a more descriptive name. Since I'm heavily depending on workers and passing them any heavy job I encounter from time to time, I want to be able to track which of them is active at the moment during runtime.

image

What is the feature you are proposing to solve the problem?

It would be great if you could allow passing an optional param to specify a name/title for the worker, so we can later on use to track which of the workers are running (by name and not by a random number).

What alternatives have you considered?

No response

@shayyzhakov shayyzhakov added the feature request Issues that request new features to be added to Node.js. label Jan 19, 2022
@targos
Copy link
Member

targos commented Jan 19, 2022

@nodejs/workers

@benjamingr
Copy link
Member

First of all - this ask sounds very reasonable to me.

Currently it just does:

  static std::string BuildWorkerTitle(int id) {
    return "Worker " + std::to_string(id);
  }

cc @eugeneo @addaleax

@targos targos added the worker Issues and PRs related to Worker support. label Jan 19, 2022
@targos
Copy link
Member

targos commented Jan 19, 2022

I wonder why the process.title and --title APis are explicitly not supported for workers.

@addaleax
Copy link
Member

@targos @benjamingr I don’t think we ever specifically thought about Worker names in the context of the inspector. #32434 also has a bunch of information about why we did not implement other kinds of APIs for naming Workers (yet).

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jul 19, 2022
@gireeshpunathil
Copy link
Member

I still believe adding name to the worker object rather than the worker's native thread (that suffers from portability issues) will be useful to some class of user scenarios - for example the tools (such as report, llnode etc.) can be taught to retrieve and display the worker names. Also, having a standard way of doing this such as in the worker constructor as opposed to the programmer explicitly adding it as a property makes the observability much more consistent.

@github-actions github-actions bot removed the stale label Jul 20, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jan 17, 2023
@flipswitchingmonkey
Copy link

Can we not just get an optional 'WorkerPrefix' parameter that falls back to 'Worker'? That seems like a rather simple change to me?

@github-actions github-actions bot removed the stale label Jan 21, 2023
@bnoordhuis
Copy link
Member

@flipswitchingmonkey I don't think there are any major objections, it just needs someone to do the work. Pull request welcome.

I'm inclined to say it should be an inspector module method but open to being convinced otherwise.

@flipswitchingmonkey
Copy link

My c++ is half a decade out of use, but I might have a look...

A quick glance at the code shows me that at

std::string name = "WorkerThread ";
we are also setting a name for the Worker thread, again hardcoded (this time to WorkerThread +id), so it would probably make sense to somehow combine those into a single customisable prefix?

@bnoordhuis
Copy link
Member

That's used for tracing. The inspector name is generated here:

info_(BuildWorkerTitle(id), url, worker_thread),

Of course it'd be good to have them be consistent. Perhaps the trace name can be computed from WorkerInfo::title or the other way around.

debadree25 added a commit to debadree25/node that referenced this issue Feb 25, 2023
debadree25 added a commit to debadree25/node that referenced this issue Feb 25, 2023
debadree25 added a commit to debadree25/node that referenced this issue Feb 28, 2023
nodejs-github-bot pushed a commit that referenced this issue Mar 6, 2023
Fixes: #41589
PR-URL: #46832
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
targos pushed a commit that referenced this issue Mar 13, 2023
Fixes: #41589
PR-URL: #46832
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
targos pushed a commit that referenced this issue Mar 14, 2023
Fixes: #41589
PR-URL: #46832
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
danielleadams pushed a commit that referenced this issue Apr 11, 2023
Fixes: #41589
PR-URL: #46832
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants