Skip to content

--cpu-prof-name 'CPU.${pid}.cpuprofile' doesn't replace the placeholder #57418

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

Open
BourgoisMickael opened this issue Mar 12, 2025 · 10 comments
Open
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors.

Comments

@BourgoisMickael
Copy link

Version

v22.13.0

Platform

Linux xps 6.8.0-52-generic #53~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Jan 15 19:18:46 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

node -p process.pid --cpu-prof --cpu-prof-name 'CPU.${pid}.cpuprofile' && ls CPU*

outputs

43689
'CPU.${pid}.cpuprofile'

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

Following the doc: https://nodejs.org/api/cli.html#--cpu-prof

If --cpu-prof-name is not specified, the generated profile is named CPU.${yyyymmdd}.${hhmmss}.${pid}.${tid}.${seq}.cpuprofile.

I expect to be able to use those placeholder when I specify a name, especially when I use the cluster module to avoid having all the profiling overriding each others.

I expected the filename to be CPU.43689.cpuprofile

What do you see instead?

'CPU.${pid}.cpuprofile'

Additional information

No response

@joyeecheung
Copy link
Member

It looks like either a misunderstanding of what the doc says, or that the doc not being clear enough that the argument is not a pattern that will be filled by Node.js. The doc specifically says:

If --cpu-prof-name is not specified, ...

which is not to be interpreted as "if it IS specified, Node.js will fill in the blank using a user-provided pattern". Rather, it means "if it IS specified, the name provided will be used as-is".

@joyeecheung joyeecheung added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Mar 12, 2025
@joyeecheung
Copy link
Member

joyeecheung commented Mar 12, 2025

labeling good first issue, I think it should be fixed by clarifying the docs that what's expected in the OP is not going to happen instead.

@BourgoisMickael
Copy link
Author

@joyeecheung could we have it as a feature to be able to use a pattern with placeholder in the name ?

Otherwise, using it with the cluster module, each process would overwrite the same file.

@cecia234
Copy link
Contributor

cecia234 commented Mar 13, 2025

@BourgoisMickael I was writing the docs and trying out the parameter myself. Calling node --cpu-prof --cpu-prof-name 'CPU.${pid}.cpuprofile' I obtain the same output as you, but calling node --cpu-prof --cpu-prof-name "CPU.${pid}.cpuprofile" (using double quotes) actually outputs a file with the pid in the name. Could you try it? Please note that I'm on windows at the moment.

EDIT: after trying it on wsl, using double quotes I get a file with the name CPU..cpuprofile, so the behaviour could be OS dependent.

@BourgoisMickael
Copy link
Author

BourgoisMickael commented Mar 13, 2025

@cecia234 The ${} syntax is a parameter substitution/expansion in shell (https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html).

Using double quotes or no quotes will trigger the substitution and replace it with the value of the variable pid or an empty string.

So on linux you end up sending --cpu-prof-name CPU..cpuprofile to the process. Hence the use of single quotes to send the literal string ${pid} as argumen without shell substitution.


It doesn't seem like a big change to be able to handle pattern as name if it's already available when name is not provided

@cecia234
Copy link
Contributor

Oof, I didn't know that I'm sorry.
I agree with you that it does not seem like a big change, I'll keep the docs PR open until a more experienced contributor will give some feedback.

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Mar 13, 2025
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Mar 13, 2025
@jasnell
Copy link
Member

jasnell commented Mar 13, 2025

I added the feature request label because it might actually be worthwhile exploring implementing this kind of replacement in the future. I agree that in the near term clarifying the docs is the best next step.

cecia234 added a commit to cecia234/node that referenced this issue Mar 14, 2025
cecia234 added a commit to cecia234/node that referenced this issue Mar 14, 2025
@priyanshscpp
Copy link

I’d like to work on this. Any guidance?

@Jatan196
Copy link

Hello there !
I'm interested to work on it, can anyone pl assign it to me?
Should I share an approach first?

jasnell pushed a commit that referenced this issue Mar 16, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this issue Mar 23, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 1, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 1, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit to RafaelGSS/node that referenced this issue Apr 8, 2025
Refs: nodejs#57418
PR-URL: nodejs#57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 14, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 14, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this issue Apr 14, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this issue Apr 14, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
aduh95 pushed a commit that referenced this issue Apr 15, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 16, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue Apr 17, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue May 1, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this issue May 2, 2025
Refs: #57418
PR-URL: #57433
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@theanarkh
Copy link
Contributor

theanarkh commented May 10, 2025

You can submit an PR directly !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

7 participants