Bug 574403 - Show Process ID in Console, Debug View and Process Properties
Summary: Show Process ID in Console, Debug View and Process Properties
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.21   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.23 M2   Edit
Assignee: Andrey Loskutov CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy, plan
Depends on:
Blocks:
 
Reported: 2021-06-23 05:28 EDT by Jörg Kubitz CLA
Modified: 2022-02-11 07:59 EST (History)
2 users (show)

See Also:


Attachments
Suggested Screenshot.png (65.39 KB, image/png)
2021-06-23 05:28 EDT, Jörg Kubitz CLA
no flags Details
Proposed change (135.74 KB, image/png)
2022-01-10 15:46 EST, Andrey Loskutov CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jörg Kubitz CLA 2021-06-23 05:28:12 EDT
Feature request:
Can we please show Process ID in Console (so even if we did not start the process in debug mode), Debug View and Process Properties. I often have to find which PID the process has to attach other tools (like profiler) to that process.
I'd be happy if someone else implement this.
Comment 1 Jörg Kubitz CLA 2021-06-23 05:28:57 EDT
Created attachment 286662 [details]
Suggested Screenshot.png
Comment 2 Sarika Sinha CLA 2021-06-23 07:14:38 EDT
This should be doable.
Comment 3 Andrey Loskutov CLA 2021-06-23 07:55:15 EDT
Please note, one launch config can host multiple processes, and not every process may have a real process behind AFAIK. At least I've saw such launches.
Comment 4 Sarika Sinha CLA 2021-06-23 12:30:57 EDT
(In reply to Andrey Loskutov from comment #3)
> Please note, one launch config can host multiple processes, and not every
> process may have a real process behind AFAIK. At least I've saw such
> launches.

Yes, but there will be still the main process id, no? 

Can you attach a sample so that it be used to test/verify for making sure that incorrect data is not displayed?
Comment 5 Andrey Loskutov CLA 2021-06-23 12:44:54 EDT
(In reply to Sarika Sinha from comment #4)
> (In reply to Andrey Loskutov from comment #3)
> > Please note, one launch config can host multiple processes, and not every
> > process may have a real process behind AFAIK. At least I've saw such
> > launches.
> 
> Yes, but there will be still the main process id, no? 

If I read this right, Launch.addProcess() allows to add an unlimited number of processes, so we can't know which one is the main one.

In the screenshot we could show pid for the whole only if there is a single process, or show pid's for every hosted process in a launch.

> Can you attach a sample so that it be used to test/verify for making sure
> that incorrect data is not displayed?

No, sorry, just happened to see that somewhere in our product where we create our own launches that represent "virtual" processes that we didn't launched from Eclipse but just connected to. 

Also I believe one could achieve that by combining multiple run configurations running e.g. external commands via a launch group.
Comment 6 Sarika Sinha CLA 2021-11-10 08:37:06 EST
@Jörg,
I was thinking of checking if Launch has only one process then show the ID else display "Multiple processes" or some msg like that.
Does that sound ok?
Comment 7 Jörg Kubitz CLA 2021-11-10 13:50:21 EST
> else display "Multiple processes" or some msg like that.
> Does that sound ok?

I would prefer a comma separated list of PIDs if there is not a strong reason against that.
Comment 8 Sarika Sinha CLA 2021-11-19 00:26:12 EST
(In reply to Jörg Kubitz from comment #7)
> > else display "Multiple processes" or some msg like that.
> > Does that sound ok?
> 
> I would prefer a comma separated list of PIDs if there is not a strong
> reason against that.

ok.
Comment 9 Sarika Sinha CLA 2022-01-10 13:22:08 EST
Adding the pid to the "Process Properties" should be sufficient as it is available even with simple Run and during runtime before termination.

Showing Process Ids in the Console will become unnecessary complicated  in the case of Group Launches where Processes are added and removed.

Any objections?
Comment 10 Andrey Loskutov CLA 2022-01-10 13:29:01 EST
Can't this be added to the process label in Debug view?
Comment 11 Sarika Sinha CLA 2022-01-10 13:57:15 EST
(In reply to Andrey Loskutov from comment #10)
> Can't this be added to the process label in Debug view?

That will mean changing the getLabel() in the RuntimeProcess and that can show up at many unwanted places as well.
Comment 12 Jörg Kubitz CLA 2022-01-10 14:02:28 EST
I still would like to see it in console and most important in debug view. And i do not need it for launch groups. Could id be just blank for groups?
just add a getLabelWithPid()?
Comment 13 Sarika Sinha CLA 2022-01-10 14:13:55 EST
(In reply to Jörg Kubitz from comment #12)
> I still would like to see it in console and most important in debug view.
> And i do not need it for launch groups. Could id be just blank for groups?
> just add a getLabelWithPid()?

Platform Debug is base for many debuggers, we have to be really careful as we don't know how these methods are being used. If we add a string to the label and some client is using the label in other places, it might be a totally unexpected and unwanted string for them.

There is nothing different about Group launches that we can restrict, they just dynamically keep changing the launches by adding//removing the processes.

Process Properties is the safest I feel.
Comment 14 Eclipse Genie CLA 2022-01-10 15:42:40 EST
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/189453
Comment 15 Andrey Loskutov CLA 2022-01-10 15:46:12 EST
Created attachment 287807 [details]
Proposed change

(In reply to Eclipse Genie from comment #14)
> New Gerrit change created:
> https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/189453

The screenshot is for the patch above, which is WIP but should show direction I'm thinking about.

I think that shouldn't be breaking change (except the quick & dirty change in DebugPlugin.newProcess() I've made because lack of time), and it should cover all three cases - Debug view, Console and Properties.

WDYT?
Comment 16 Sarika Sinha CLA 2022-01-11 02:06:59 EST
(In reply to Andrey Loskutov from comment #15)
> Created attachment 287807 [details]
> Proposed change
> 
> (In reply to Eclipse Genie from comment #14)
> > New Gerrit change created:
> > https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/189453
> 
> The screenshot is for the patch above, which is WIP but should show
> direction I'm thinking about.
> 
> I think that shouldn't be breaking change (except the quick & dirty change
> in DebugPlugin.newProcess() I've made because lack of time), and it should
> cover all three cases - Debug view, Console and Properties.
> 
> WDYT?

This seems ok, more of a localized change.
Comment 17 Sarika Sinha CLA 2022-01-24 11:29:56 EST
@Andrey, Will you be completing the WIP gerrit or I should do it?
Comment 18 Eclipse Genie CLA 2022-01-24 14:13:12 EST
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/189972
Comment 19 Andrey Loskutov CLA 2022-01-24 14:33:02 EST
(In reply to Sarika Sinha from comment #17)
> @Andrey, Will you be completing the WIP gerrit or I should do it?

Should be OK for review now.
Comment 20 Sarika Sinha CLA 2022-01-25 14:43:14 EST
(In reply to Eclipse Genie from comment #18)
> New Gerrit change created:
> https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/189972

This will add the pid only in the debug view ?

Platform Debug gerrit
Comment 21 Andrey Loskutov CLA 2022-01-25 15:08:38 EST
(In reply to Sarika Sinha from comment #20)
> (In reply to Eclipse Genie from comment #18)
> > New Gerrit change created:
> > https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/189972
> 
> This will add the pid only in the debug view ?

Yep.
Comment 24 Eclipse Genie CLA 2022-01-26 09:51:05 EST
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/190044
Comment 25 Jörg Kubitz CLA 2022-01-26 10:10:09 EST
thanks Andrey!!
Comment 27 Andrey Loskutov CLA 2022-01-26 10:27:11 EST
Verified with build I20220125-1800
Comment 28 Sarika Sinha CLA 2022-01-27 04:45:56 EST
(In reply to Eclipse Genie from comment #24)
> New Gerrit change created:
> https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/190044

Can we add the image of Process Properties page also?
Comment 29 Eclipse Genie CLA 2022-02-11 07:56:41 EST
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/190716
Comment 31 Andrey Loskutov CLA 2022-02-11 07:59:22 EST
(In reply to Sarika Sinha from comment #28)
> Can we add the image of Process Properties page also?

Done.