diff mbox series

[v6,2/2] tr2: log parent process name

Message ID 20210722012707.205776-3-emilyshaffer@google.com (mailing list archive)
State Accepted
Commit 2f732bf15e6dc9c2caf210784f180c6c059c570a
Headers show
Series tr2: log parent process name | expand

Commit Message

Emily Shaffer July 22, 2021, 1:27 a.m. UTC
It can be useful to tell who invoked Git - was it invoked manually by a
user via CLI or script? By an IDE?  In some cases - like 'repo' tool -
we can influence the source code and set the GIT_TRACE2_PARENT_SID
environment variable from the caller process. In 'repo''s case, that
parent SID is manipulated to include the string "repo", which means we
can positively identify when Git was invoked by 'repo' tool. However,
identifying parents that way requires both that we know which tools
invoke Git and that we have the ability to modify the source code of
those tools. It cannot scale to keep up with the various IDEs and
wrappers which use Git, most of which we don't know about. Learning
which tools and wrappers invoke Git, and how, would give us insight to
decide where to improve Git's usability and performance.

Unfortunately, there's no cross-platform reliable way to gather the name
of the parent process. If procfs is present, we can use that; otherwise
we will need to discover the name another way. However, the process ID
should be sufficient to look up the process name on most platforms, so
that code may be shareable.

Git for Windows gathers similar information and logs it as a "data_json"
event. However, since "data_json" has a variable format, it is difficult
to parse effectively in some languages; instead, let's pursue a
dedicated "cmd_ancestry" event to record information about the ancestry
of the current process and a consistent, parseable way.

Git for Windows also gathers information about more than one generation
of parent. In Linux further ancestry info can be gathered with procfs,
but it's unwieldy to do so. In the interest of later moving Git for
Windows ancestry logging to the 'cmd_ancestry' event, and in the
interest of later adding more ancestry to the Linux implementation - or
of adding this functionality to other platforms which have an easier
time walking the process tree - let's make 'cmd_ancestry' accept an
array of parentage.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---

Notes:
    Since v3:
    
    Junio and Randall suggested ditching "get_process_name(ppid)" as the API to
    implement on various platforms, and I liked the suggestion a lot. So instead,
    I added "get_ancestry_names(strvec *names)".
    
     - Using a strvec instead of a char** makes cleanup easier, since strvec takes
       care of counting the number of strings in the array for us. Otherwise we'd
       need to include size as a return or out-param in get_ancestry_names(), and
       that's what the util class is for, right? :)
     - I made get_ancestry_names() static instead of putting it into
       git-compat-util.h. I think I had put get_process_name() into that header to
       facilitate non-procfs implementations, but compat/procinfo.c doesn't seem to
       me to indicate "procfs only", and if we do need to implement
       get_process_name() somewhere else, it'll be pretty easy to move it.
     - I added a description of "cmd_ancestry" to
       Documentation/technical/api-trace2.txt. I didn't see any user-facing docs to
       update (for example, "git grep cmd_path" produces only that one doc file).
    
    Thanks, all.
    
     - Emily

 Documentation/technical/api-trace2.txt | 14 +++++++
 compat/linux/procinfo.c                | 55 ++++++++++++++++++++++++++
 config.mak.uname                       |  2 +
 t/t0210/scrub_normal.perl              |  6 +++
 t/t0211/scrub_perf.perl                |  5 +++
 t/t0212/parse_events.perl              |  5 ++-
 trace2.c                               | 13 ++++++
 trace2.h                               | 10 +++++
 trace2/tr2_tgt.h                       |  3 ++
 trace2/tr2_tgt_event.c                 | 21 ++++++++++
 trace2/tr2_tgt_normal.c                | 19 +++++++++
 trace2/tr2_tgt_perf.c                  | 16 ++++++++
 12 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 compat/linux/procinfo.c

Comments

Junio C Hamano July 22, 2021, 9:02 p.m. UTC | #1
Emily Shaffer <emilyshaffer@google.com> writes:

> Unfortunately, there's no cross-platform reliable way to gather the name
> of the parent process. If procfs is present, we can use that; otherwise
> we will need to discover the name another way. However, the process ID
> should be sufficient to look up the process name on most platforms, so
> that code may be shareable.

Is the sentence that begin with "However" still relevant?  The
latest get_ancestry_names() does not add anything when the result
read from the procfs is unusable, but the sentence gives a false
impression that it may somehow fall back to show the PID.

> Git for Windows also gathers information about more than one generation
> of parent. In Linux further ancestry info can be gathered with procfs,
> but it's unwieldy to do so. In the interest of later moving Git for
> Windows ancestry logging to the 'cmd_ancestry' event, and in the
> interest of later adding more ancestry to the Linux implementation - or
> of adding this functionality to other platforms which have an easier
> time walking the process tree - let's make 'cmd_ancestry' accept an
> array of parentage.

Clearly written.  Nice.

> +`"cmd_ancestry"`::
> +	This event contains the text command name for the parent (and earlier
> +	generations of parents) of the current process, in an array ordered from
> +	nearest parent to furthest great-grandparent. It may not be implemented
> +	on all platforms.
> ++
> +------------
> +{
> +	"event":"cmd_ancestry",
> +	...
> +	"ancestry":["bash","tmux: server","systemd"]
> +}
> +------------

OK.

> diff --git a/compat/linux/procinfo.c b/compat/linux/procinfo.c
> new file mode 100644
> index 0000000000..578fed4cd3
> --- /dev/null
> +++ b/compat/linux/procinfo.c
> @@ -0,0 +1,55 @@
> +#include "cache.h"
> +
> +#include "strbuf.h"
> +#include "strvec.h"
> +#include "trace2.h"
> +
> +static void get_ancestry_names(struct strvec *names)
> +{
> +	/*
> +	 * NEEDSWORK: We could gather the entire pstree into an array to match
> +	 * functionality with compat/win32/trace2_win32_process_info.c.
> +	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
> +	 * gather the immediate parent name which is readily accessible from
> +	 * /proc/$(getppid())/comm.
> +	 */
> +	struct strbuf procfs_path = STRBUF_INIT;
> +	struct strbuf name = STRBUF_INIT;
> +
> +	/* try to use procfs if it's present. */
> +	strbuf_addf(&procfs_path, "/proc/%d/comm", getppid());
> +	if (strbuf_read_file(&name, procfs_path.buf, 0)) {
> +		strbuf_release(&procfs_path);
> +		strbuf_trim_trailing_newline(&name);
> +		strvec_push(names, strbuf_detach(&name, NULL));

At this point, we successfully read from /proc/$(ppid)/comm
and pushed the result into names strvec.  I think you would want to
have an explicit "return;" here.

Alternatively, you could put the rest of the function in the
corresponding "else" clause, but I think an early return after each
method successfully collects the result would make a lot more sense.

> +	}
> +
> +	return;
> +	/* NEEDSWORK: add non-procfs-linux implementations here */

This looks the other way around.  A future non-procfs-linux
implementation written here will be unreachable code ;-)

Just lose "return;" from here, have one in the if(){} body, and keep
the "here is where you add more/other implementations" comment and
we'd be OK, I think.

> +void trace2_collect_process_info(enum trace2_process_info_reason reason)
> +{
> +	if (!trace2_is_enabled())
> +		return;
> +
> +	/* someday we may want to write something extra here, but not today */
> +	if (reason == TRACE2_PROCESS_INFO_EXIT)
> +		return;
> +
> +	if (reason == TRACE2_PROCESS_INFO_STARTUP) {
> +		/*
> +		 * NEEDSWORK: we could do the entire ptree in an array instead,
> +		 * see compat/win32/trace2_win32_process_info.c.
> +		 */
> +		struct strvec names = STRVEC_INIT;
> +
> +		get_ancestry_names(&names);
> +
> +		if (names.nr)
> +			trace2_cmd_ancestry(names.v);
> +		strvec_clear(&names);
> +	}
> +
> +	return;
> +}

Good.

Thanks.
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 9:38 a.m. UTC | #2
On Wed, Jul 21 2021, Emily Shaffer wrote:

> +`"cmd_ancestry"`::
> +	This event contains the text command name for the parent (and earlier
> +	generations of parents) of the current process, in an array ordered from
> +	nearest parent to furthest great-grandparent. It may not be implemented
> +	on all platforms.
> ++
> +------------
> +{
> +	"event":"cmd_ancestry",
> +	...
> +	"ancestry":["bash","tmux: server","systemd"]
> +}
> +------------
> +

Okey, so because of later NEEDSWORK no system that runs systemd will
currrently have output like this, just Windows.

> +	/*
> +	 * NEEDSWORK: We could gather the entire pstree into an array to match
> +	 * functionality with compat/win32/trace2_win32_process_info.c.
> +	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
> +	 * gather the immediate parent name which is readily accessible from
> +	 * /proc/$(getppid())/comm.
> +	 */

This comment:

> +	struct strbuf procfs_path = STRBUF_INIT;
> +	struct strbuf name = STRBUF_INIT;
> +
> +	/* try to use procfs if it's present. */
> +	strbuf_addf(&procfs_path, "/proc/%d/comm", getppid());
> +	if (strbuf_read_file(&name, procfs_path.buf, 0)) {
> +		strbuf_release(&procfs_path);
> +		strbuf_trim_trailing_newline(&name);
> +		strvec_push(names, strbuf_detach(&name, NULL));
> +	}
> +
> +	return;
> +	/* NEEDSWORK: add non-procfs-linux implementations here */
> +}
> +
> +void trace2_collect_process_info(enum trace2_process_info_reason reason)
> +{
> +	if (!trace2_is_enabled())
> +		return;
> +
> +	/* someday we may want to write something extra here, but not today */
> +	if (reason == TRACE2_PROCESS_INFO_EXIT)
> +		return;
> +
> +	if (reason == TRACE2_PROCESS_INFO_STARTUP) {

This should be a switch/case, so we get the compiler asserting/warning
if we don't check enum arms, in this case there's just these two, let's
have that be clear for readability.

> +		/*
> +		 * NEEDSWORK: we could do the entire ptree in an array instead,
> +		 * see compat/win32/trace2_win32_process_info.c.
> +		 */

Seems to remove the need for this comment, i.e. we comment on this
limitation of linux-specific get_ancestry_names twice, both in the
function and its caller.

> -    
> +    elsif ($event eq 'cmd_ancestry') {
> +	# 'cmd_ancestry' is platform-specific and not implemented everywhere, so
> +	# just skip it for testing purposes.
> +    }

The rest of this code uses two "\n" between elsif arms, let's be
consistent here.
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 10:22 a.m. UTC | #3
On Wed, Jul 21 2021, Emily Shaffer wrote:

> Git for Windows also gathers information about more than one generation
> of parent. In Linux further ancestry info can be gathered with procfs,
> but it's unwieldy to do so.

Having read the win32 get_processes() implementation and read proc(5) I
don't get how it's unweildy to do so on Linux? Perhaps I'm missing some
special-case but this rather simple patch-on-top seems to do the job for
me. This includes the unrelated enum/switch/case change I suggested.

I can submit it as a patch-on-top with SOB etc, but maybe there's some
subtle reason it won't work properly. It works for me, I get e.g.:
    
    {
      "event": "cmd_ancestry",
      "sid": "20210802T102731.879424Z-Hc2f5b994-P00001acc",
      "thread": "main",
      "time": "2021-08-02T10:27:31.879618Z",
      "file": "compat/linux/procinfo.c",
      "line": 66,
      "ancestry": [
        "bash",
        "screen",
        "systemd"
      ]
    }

If anything the existing Windows version seems a bit unweildy, having to
apparently deal with cycles etc., I don't think that happens under
procfs, but maybe I'm missing some special-case.

diff --git a/compat/linux/procinfo.c b/compat/linux/procinfo.c
index 578fed4cd31..671fe2395fd 100644
--- a/compat/linux/procinfo.c
+++ b/compat/linux/procinfo.c
@@ -4,51 +4,65 @@
 #include "strvec.h"
 #include "trace2.h"
 
-static void get_ancestry_names(struct strvec *names)
+static int stat_parent_pid(FILE *fp, char *statcomm, int *statppid)
+{
+	char statstate;
+	int statpid;
+
+	int ret = fscanf(fp, "%d %s %c %d", &statpid, statcomm, &statstate,
+			 statppid);
+	if (ret != 4)
+		return -1;
+	return 0;
+}
+
+static void push_ancestry_name(struct strvec *names, pid_t pid)
 {
-	/*
-	 * NEEDSWORK: We could gather the entire pstree into an array to match
-	 * functionality with compat/win32/trace2_win32_process_info.c.
-	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
-	 * gather the immediate parent name which is readily accessible from
-	 * /proc/$(getppid())/comm.
-	 */
 	struct strbuf procfs_path = STRBUF_INIT;
-	struct strbuf name = STRBUF_INIT;
+	char statcomm[PATH_MAX];
+	FILE *fp;
+	int ppid;
 
 	/* try to use procfs if it's present. */
-	strbuf_addf(&procfs_path, "/proc/%d/comm", getppid());
-	if (strbuf_read_file(&name, procfs_path.buf, 0)) {
-		strbuf_release(&procfs_path);
-		strbuf_trim_trailing_newline(&name);
-		strvec_push(names, strbuf_detach(&name, NULL));
-	}
+	strbuf_addf(&procfs_path, "/proc/%d/stat", pid);
+	fp = fopen(procfs_path.buf, "r");
+	if (!fp)
+		return;
 
-	return;
-	/* NEEDSWORK: add non-procfs-linux implementations here */
+	if (stat_parent_pid(fp, statcomm, &ppid) < 0)
+		return;
+
+	/*
+	 * The comm field is in parenthesis, use printf + offset as a
+	 * poor man's trimming of both ends.
+	 */
+	strvec_pushf(names, "%.*s", (int)strlen(statcomm) - 2, statcomm + 1);
+
+	/*
+	 * Both errors and reaching the end of the process chain are
+	 * reported as fields of 0 by proc(5)
+	 */
+	if (ppid != 0)
+		push_ancestry_name(names, ppid);
 }
 
 void trace2_collect_process_info(enum trace2_process_info_reason reason)
 {
-	if (!trace2_is_enabled())
-		return;
+	struct strvec names = STRVEC_INIT;
 
-	/* someday we may want to write something extra here, but not today */
-	if (reason == TRACE2_PROCESS_INFO_EXIT)
+	if (!trace2_is_enabled())
 		return;
 
-	if (reason == TRACE2_PROCESS_INFO_STARTUP) {
-		/*
-		 * NEEDSWORK: we could do the entire ptree in an array instead,
-		 * see compat/win32/trace2_win32_process_info.c.
-		 */
-		struct strvec names = STRVEC_INIT;
-
-		get_ancestry_names(&names);
+	switch (reason) {
+	case TRACE2_PROCESS_INFO_EXIT:
+		break;
+	case TRACE2_PROCESS_INFO_STARTUP:
+		push_ancestry_name(&names, getppid());
 
 		if (names.nr)
 			trace2_cmd_ancestry(names.v);
 		strvec_clear(&names);
+		break;
 	}
 
 	return;
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 10:30 a.m. UTC | #4
On Wed, Jul 21 2021, Emily Shaffer wrote:

>  compat/linux/procinfo.c                | 55 ++++++++++++++++++++++++++
> [...]
> +	/* NEEDSWORK: add non-procfs-linux implementations here */

We'd want non-Windows and non-Linux implementations of this, but that
NEEDSWORK comment should not be in compat/linux/procinfo.c, where we're
never going to add non-Linux code.
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 12:45 p.m. UTC | #5
On Mon, Aug 02 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Jul 21 2021, Emily Shaffer wrote:
>
>> +`"cmd_ancestry"`::
>> +	This event contains the text command name for the parent (and earlier
>> +	generations of parents) of the current process, in an array ordered from
>> +	nearest parent to furthest great-grandparent. It may not be implemented
>> +	on all platforms.
>> ++
>> +------------
>> +{
>> +	"event":"cmd_ancestry",
>> +	...
>> +	"ancestry":["bash","tmux: server","systemd"]
>> +}
>> +------------
>> +
>
> Okey, so because of later NEEDSWORK no system that runs systemd will
> currrently have output like this, just Windows.
>
>> +	/*
>> +	 * NEEDSWORK: We could gather the entire pstree into an array to match
>> +	 * functionality with compat/win32/trace2_win32_process_info.c.
>> +	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
>> +	 * gather the immediate parent name which is readily accessible from
>> +	 * /proc/$(getppid())/comm.
>> +	 */
>
> This comment:

To clarify, this was ment to be continued with "seems to remove the
need..." below. But I later inserted the commentery about switch/case
in-between, which made that unclear.

> [...]
>> [...]
>> +		/*
>> +		 * NEEDSWORK: we could do the entire ptree in an array instead,
>> +		 * see compat/win32/trace2_win32_process_info.c.
>> +		 */
>
> Seems to remove the need for this comment, i.e. we comment on this
> limitation of linux-specific get_ancestry_names twice, both in the
> function and its caller.
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 12:47 p.m. UTC | #6
On Mon, Aug 02 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Jul 21 2021, Emily Shaffer wrote:
>
>> Git for Windows also gathers information about more than one generation
>> of parent. In Linux further ancestry info can be gathered with procfs,
>> but it's unwieldy to do so.
>
> Having read the win32 get_processes() implementation and read proc(5) I
> don't get how it's unweildy to do so on Linux? Perhaps I'm missing some
> special-case but this rather simple patch-on-top seems to do the job for
> me. This includes the unrelated enum/switch/case change I suggested.
>
> I can submit it as a patch-on-top with SOB etc, but maybe there's some
> subtle reason it won't work properly. It works for me, I get e.g.:

Just a note[...]:


> +	strbuf_addf(&procfs_path, "/proc/%d/stat", pid);
> +	fp = fopen(procfs_path.buf, "r");
> +	if (!fp)
> +		return;

I think this is mostly ready, but is still clearly in POC state needing
polishing, e.g. I didn't bother to close "fp" here, but a final patch to
implement this approach should of course do that.
Jeff Hostetler Aug. 2, 2021, 3:23 p.m. UTC | #7
On 8/2/21 6:22 AM, Ævar Arnfjörð Bjarmason wrote:
...
> 
> If anything the existing Windows version seems a bit unweildy, having to
> apparently deal with cycles etc., I don't think that happens under
> procfs, but maybe I'm missing some special-case.
> 

It's been a while since I did the parent lookup on Windows, but...

Yes, on Windows we observed cases where a PID would be recycled
and a child process could have the same PID as an ancient ancestor
that had long since exited.

I don't think the cycle problem exists on Unix because IIRC a
child process' parent-PID is set to Init when the parent exits.

Windows does not update the child's parent-PID when the parent
exits, so if a recently-used PID is given to one of our child
processes, we can get a loop.

Jeff
Randall S. Becker Aug. 2, 2021, 4:10 p.m. UTC | #8
On August 2, 2021 6:22 AM: Ævar Arnfjörð Bjarmason wroteL
>On Wed, Jul 21 2021, Emily Shaffer wrote:
>
>> Git for Windows also gathers information about more than one
>> generation of parent. In Linux further ancestry info can be gathered
>> with procfs, but it's unwieldy to do so.
>
>Having read the win32 get_processes() implementation and read proc(5) I don't get how it's unweildy to do so on Linux? Perhaps I'm
>missing some special-case but this rather simple patch-on-top seems to do the job for me. This includes the unrelated enum/switch/case
>change I suggested.
>
>I can submit it as a patch-on-top with SOB etc, but maybe there's some subtle reason it won't work properly. It works for me, I get e.g.:
>
>    {
>      "event": "cmd_ancestry",
>      "sid": "20210802T102731.879424Z-Hc2f5b994-P00001acc",
>      "thread": "main",
>      "time": "2021-08-02T10:27:31.879618Z",
>      "file": "compat/linux/procinfo.c",
>      "line": 66,
>      "ancestry": [
>        "bash",
>        "screen",
>        "systemd"
>      ]
>    }

Should not the subfields of "ancestry" also have field names? I get that they are a list, but it seems a bit restrictive.

My preference here would be:

     "ancestry": [
       "ancestor": [
	"program": "bash",
	"pid" : 1234 ],
       "ancestor": [
              "program": "screen"],
       "ancestor": [
       	"program" : "systemd"],
     ]

With more richness available in the ancestor.
Junio C Hamano Aug. 2, 2021, 4:24 p.m. UTC | #9
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Wed, Jul 21 2021, Emily Shaffer wrote:
>
>>  compat/linux/procinfo.c                | 55 ++++++++++++++++++++++++++
>> [...]
>> +	/* NEEDSWORK: add non-procfs-linux implementations here */
>
> We'd want non-Windows and non-Linux implementations of this, but that
> NEEDSWORK comment should not be in compat/linux/procinfo.c, where we're
> never going to add non-Linux code.

I am puzzled.  This is talking about additional implementation for
Linux that does not use procfs, no (i.e. what to do with builds of
Linux that compile out the procfs support or installations that do
not mount the /proc hierarchy)?

The comment seems to be at the right place to remind us of them,
even though I do not know how important such an environment is.
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 6:41 p.m. UTC | #10
On Mon, Aug 02 2021, Randall S. Becker wrote:

> On August 2, 2021 6:22 AM: Ævar Arnfjörð Bjarmason wroteL
>>On Wed, Jul 21 2021, Emily Shaffer wrote:
>>
>>> Git for Windows also gathers information about more than one
>>> generation of parent. In Linux further ancestry info can be gathered
>>> with procfs, but it's unwieldy to do so.
>>
>>Having read the win32 get_processes() implementation and read proc(5) I don't get how it's unweildy to do so on Linux? Perhaps I'm
>>missing some special-case but this rather simple patch-on-top seems to do the job for me. This includes the unrelated enum/switch/case
>>change I suggested.
>>
>>I can submit it as a patch-on-top with SOB etc, but maybe there's some subtle reason it won't work properly. It works for me, I get e.g.:
>>
>>    {
>>      "event": "cmd_ancestry",
>>      "sid": "20210802T102731.879424Z-Hc2f5b994-P00001acc",
>>      "thread": "main",
>>      "time": "2021-08-02T10:27:31.879618Z",
>>      "file": "compat/linux/procinfo.c",
>>      "line": 66,
>>      "ancestry": [
>>        "bash",
>>        "screen",
>>        "systemd"
>>      ]
>>    }
>
> Should not the subfields of "ancestry" also have field names? I get that they are a list, but it seems a bit restrictive.
>
> My preference here would be:
>
>      "ancestry": [
>        "ancestor": [
> 	"program": "bash",
> 	"pid" : 1234 ],
>        "ancestor": [
>               "program": "screen"],
>        "ancestor": [
>        	"program" : "systemd"],
>      ]
>
> With more richness available in the ancestor.

That sounds sensible, but to be clear that's a relevant comment on
Emily's original patch, my "let's implement the same for Linux" is just
faithfully reproducing what we're already doing in the Windows
implementation.

But yes, I'd think that including the PID would be a sensible thing to
do...
Ævar Arnfjörð Bjarmason Aug. 2, 2021, 6:42 p.m. UTC | #11
On Mon, Aug 02 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Wed, Jul 21 2021, Emily Shaffer wrote:
>>
>>>  compat/linux/procinfo.c                | 55 ++++++++++++++++++++++++++
>>> [...]
>>> +	/* NEEDSWORK: add non-procfs-linux implementations here */
>>
>> We'd want non-Windows and non-Linux implementations of this, but that
>> NEEDSWORK comment should not be in compat/linux/procinfo.c, where we're
>> never going to add non-Linux code.
>
> I am puzzled.  This is talking about additional implementation for
> Linux that does not use procfs, no (i.e. what to do with builds of
> Linux that compile out the procfs support or installations that do
> not mount the /proc hierarchy)?
>
> The comment seems to be at the right place to remind us of them,
> even though I do not know how important such an environment is.

Yes, I see I misread that, nevermind the narrow suggestion then.

Is there a way to do this sort of thing on Linux without procfs? Other
than things that use procfs themselves, e.g. parsing "ps" output.
diff mbox series

Patch

diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
index 3f52f981a2..8a0b360a0e 100644
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -493,6 +493,20 @@  about specific error arguments.
 }
 ------------
 
+`"cmd_ancestry"`::
+	This event contains the text command name for the parent (and earlier
+	generations of parents) of the current process, in an array ordered from
+	nearest parent to furthest great-grandparent. It may not be implemented
+	on all platforms.
++
+------------
+{
+	"event":"cmd_ancestry",
+	...
+	"ancestry":["bash","tmux: server","systemd"]
+}
+------------
+
 `"cmd_name"`::
 	This event contains the command name for this git process
 	and the hierarchy of commands from parent git processes.
diff --git a/compat/linux/procinfo.c b/compat/linux/procinfo.c
new file mode 100644
index 0000000000..578fed4cd3
--- /dev/null
+++ b/compat/linux/procinfo.c
@@ -0,0 +1,55 @@ 
+#include "cache.h"
+
+#include "strbuf.h"
+#include "strvec.h"
+#include "trace2.h"
+
+static void get_ancestry_names(struct strvec *names)
+{
+	/*
+	 * NEEDSWORK: We could gather the entire pstree into an array to match
+	 * functionality with compat/win32/trace2_win32_process_info.c.
+	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
+	 * gather the immediate parent name which is readily accessible from
+	 * /proc/$(getppid())/comm.
+	 */
+	struct strbuf procfs_path = STRBUF_INIT;
+	struct strbuf name = STRBUF_INIT;
+
+	/* try to use procfs if it's present. */
+	strbuf_addf(&procfs_path, "/proc/%d/comm", getppid());
+	if (strbuf_read_file(&name, procfs_path.buf, 0)) {
+		strbuf_release(&procfs_path);
+		strbuf_trim_trailing_newline(&name);
+		strvec_push(names, strbuf_detach(&name, NULL));
+	}
+
+	return;
+	/* NEEDSWORK: add non-procfs-linux implementations here */
+}
+
+void trace2_collect_process_info(enum trace2_process_info_reason reason)
+{
+	if (!trace2_is_enabled())
+		return;
+
+	/* someday we may want to write something extra here, but not today */
+	if (reason == TRACE2_PROCESS_INFO_EXIT)
+		return;
+
+	if (reason == TRACE2_PROCESS_INFO_STARTUP) {
+		/*
+		 * NEEDSWORK: we could do the entire ptree in an array instead,
+		 * see compat/win32/trace2_win32_process_info.c.
+		 */
+		struct strvec names = STRVEC_INIT;
+
+		get_ancestry_names(&names);
+
+		if (names.nr)
+			trace2_cmd_ancestry(names.v);
+		strvec_clear(&names);
+	}
+
+	return;
+}
diff --git a/config.mak.uname b/config.mak.uname
index 185ff79b14..d3bd4c6843 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -58,6 +58,8 @@  ifeq ($(uname_S),Linux)
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	BASIC_CFLAGS += -DHAVE_SYSINFO
 	PROCFS_EXECUTABLE_PATH = /proc/self/exe
+	HAVE_PLATFORM_PROCINFO = YesPlease
+	COMPAT_OBJS += compat/linux/procinfo.o
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
diff --git a/t/t0210/scrub_normal.perl b/t/t0210/scrub_normal.perl
index c65d1a815e..7cc4de392a 100644
--- a/t/t0210/scrub_normal.perl
+++ b/t/t0210/scrub_normal.perl
@@ -42,6 +42,12 @@ 
 	# so just omit it for testing purposes.
 	# print "cmd_path _EXE_\n";
     }
+    elsif ($line =~ m/^cmd_ancestry/) {
+	# 'cmd_ancestry' is not implemented everywhere, so for portability's
+	# sake, skip it when parsing normal.
+	#
+	# print "$line";
+    }
     else {
 	print "$line";
     }
diff --git a/t/t0211/scrub_perf.perl b/t/t0211/scrub_perf.perl
index 351af7844e..d164b750ff 100644
--- a/t/t0211/scrub_perf.perl
+++ b/t/t0211/scrub_perf.perl
@@ -44,6 +44,11 @@ 
 	# $tokens[$col_rest] = "_EXE_";
 	goto SKIP_LINE;
     }
+    elsif ($tokens[$col_event] =~ m/cmd_ancestry/) {
+	# 'cmd_ancestry' is platform-specific and not implemented everywhere,
+	# so skip it.
+	goto SKIP_LINE;
+    }
     elsif ($tokens[$col_event] =~ m/child_exit/) {
 	$tokens[$col_rest] =~ s/ pid:\d* / pid:_PID_ /;
     }
diff --git a/t/t0212/parse_events.perl b/t/t0212/parse_events.perl
index 6584bb5634..b6408560c0 100644
--- a/t/t0212/parse_events.perl
+++ b/t/t0212/parse_events.perl
@@ -132,7 +132,10 @@ 
 	# just omit it for testing purposes.
 	# $processes->{$sid}->{'path'} = "_EXE_";
     }
-    
+    elsif ($event eq 'cmd_ancestry') {
+	# 'cmd_ancestry' is platform-specific and not implemented everywhere, so
+	# just skip it for testing purposes.
+    }
     elsif ($event eq 'cmd_name') {
 	$processes->{$sid}->{'name'} = $line->{'name'};
 	$processes->{$sid}->{'hierarchy'} = $line->{'hierarchy'};
diff --git a/trace2.c b/trace2.c
index 256120c7fd..b9b154ac44 100644
--- a/trace2.c
+++ b/trace2.c
@@ -260,6 +260,19 @@  void trace2_cmd_path_fl(const char *file, int line, const char *pathname)
 			tgt_j->pfn_command_path_fl(file, line, pathname);
 }
 
+void trace2_cmd_ancestry_fl(const char *file, int line, const char **parent_names)
+{
+	struct tr2_tgt *tgt_j;
+	int j;
+
+	if (!trace2_enabled)
+		return;
+
+	for_each_wanted_builtin (j, tgt_j)
+		if (tgt_j->pfn_command_ancestry_fl)
+			tgt_j->pfn_command_ancestry_fl(file, line, parent_names);
+}
+
 void trace2_cmd_name_fl(const char *file, int line, const char *name)
 {
 	struct tr2_tgt *tgt_j;
diff --git a/trace2.h b/trace2.h
index 0d990db817..9b7286c572 100644
--- a/trace2.h
+++ b/trace2.h
@@ -133,6 +133,16 @@  void trace2_cmd_path_fl(const char *file, int line, const char *pathname);
 
 #define trace2_cmd_path(p) trace2_cmd_path_fl(__FILE__, __LINE__, (p))
 
+/*
+ * Emit an 'ancestry' event with the process name of the current process's
+ * parent process.
+ * This gives post-processors a way to determine what invoked the command and
+ * learn more about usage patterns.
+ */
+void trace2_cmd_ancestry_fl(const char *file, int line, const char **parent_names);
+
+#define trace2_cmd_ancestry(v) trace2_cmd_ancestry_fl(__FILE__, __LINE__, (v))
+
 /*
  * Emit a 'cmd_name' event with the canonical name of the command.
  * This gives post-processors a simple field to identify the command
diff --git a/trace2/tr2_tgt.h b/trace2/tr2_tgt.h
index 7b90469212..1f66fd6573 100644
--- a/trace2/tr2_tgt.h
+++ b/trace2/tr2_tgt.h
@@ -27,6 +27,8 @@  typedef void(tr2_tgt_evt_error_va_fl_t)(const char *file, int line,
 
 typedef void(tr2_tgt_evt_command_path_fl_t)(const char *file, int line,
 					    const char *command_path);
+typedef void(tr2_tgt_evt_command_ancestry_fl_t)(const char *file, int line,
+						const char **parent_names);
 typedef void(tr2_tgt_evt_command_name_fl_t)(const char *file, int line,
 					    const char *name,
 					    const char *hierarchy);
@@ -108,6 +110,7 @@  struct tr2_tgt {
 	tr2_tgt_evt_atexit_t                    *pfn_atexit;
 	tr2_tgt_evt_error_va_fl_t               *pfn_error_va_fl;
 	tr2_tgt_evt_command_path_fl_t           *pfn_command_path_fl;
+	tr2_tgt_evt_command_ancestry_fl_t	*pfn_command_ancestry_fl;
 	tr2_tgt_evt_command_name_fl_t           *pfn_command_name_fl;
 	tr2_tgt_evt_command_mode_fl_t           *pfn_command_mode_fl;
 	tr2_tgt_evt_alias_fl_t                  *pfn_alias_fl;
diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index 6353e8ad91..578a9a5287 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c
@@ -261,6 +261,26 @@  static void fn_command_path_fl(const char *file, int line, const char *pathname)
 	jw_release(&jw);
 }
 
+static void fn_command_ancestry_fl(const char *file, int line, const char **parent_names)
+{
+	const char *event_name = "cmd_ancestry";
+	const char *parent_name = NULL;
+	struct json_writer jw = JSON_WRITER_INIT;
+
+	jw_object_begin(&jw, 0);
+	event_fmt_prepare(event_name, file, line, NULL, &jw);
+	jw_object_inline_begin_array(&jw, "ancestry");
+
+	while ((parent_name = *parent_names++))
+		jw_array_string(&jw, parent_name);
+
+	jw_end(&jw); /* 'ancestry' array */
+	jw_end(&jw); /* event object */
+
+	tr2_dst_write_line(&tr2dst_event, &jw.json);
+	jw_release(&jw);
+}
+
 static void fn_command_name_fl(const char *file, int line, const char *name,
 			       const char *hierarchy)
 {
@@ -584,6 +604,7 @@  struct tr2_tgt tr2_tgt_event = {
 	fn_atexit,
 	fn_error_va_fl,
 	fn_command_path_fl,
+	fn_command_ancestry_fl,
 	fn_command_name_fl,
 	fn_command_mode_fl,
 	fn_alias_fl,
diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index 31b602c171..a5751c8864 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -160,6 +160,24 @@  static void fn_command_path_fl(const char *file, int line, const char *pathname)
 	strbuf_release(&buf_payload);
 }
 
+static void fn_command_ancestry_fl(const char *file, int line, const char **parent_names)
+{
+	const char *parent_name = NULL;
+	struct strbuf buf_payload = STRBUF_INIT;
+
+	/* cmd_ancestry parent <- grandparent <- great-grandparent */
+	strbuf_addstr(&buf_payload, "cmd_ancestry ");
+	while ((parent_name = *parent_names++)) {
+		strbuf_addstr(&buf_payload, parent_name);
+		/* if we'll write another one after this, add a delimiter */
+		if (parent_names && *parent_names)
+			strbuf_addstr(&buf_payload, " <- ");
+	}
+
+	normal_io_write_fl(file, line, &buf_payload);
+	strbuf_release(&buf_payload);
+}
+
 static void fn_command_name_fl(const char *file, int line, const char *name,
 			       const char *hierarchy)
 {
@@ -306,6 +324,7 @@  struct tr2_tgt tr2_tgt_normal = {
 	fn_atexit,
 	fn_error_va_fl,
 	fn_command_path_fl,
+	fn_command_ancestry_fl,
 	fn_command_name_fl,
 	fn_command_mode_fl,
 	fn_alias_fl,
diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c
index a8018f18cc..af4d65a0a5 100644
--- a/trace2/tr2_tgt_perf.c
+++ b/trace2/tr2_tgt_perf.c
@@ -253,6 +253,21 @@  static void fn_command_path_fl(const char *file, int line, const char *pathname)
 	strbuf_release(&buf_payload);
 }
 
+static void fn_command_ancestry_fl(const char *file, int line, const char **parent_names)
+{
+	const char *event_name = "cmd_ancestry";
+	struct strbuf buf_payload = STRBUF_INIT;
+
+	strbuf_addstr(&buf_payload, "ancestry:[");
+	/* It's not an argv but the rules are basically the same. */
+	sq_append_quote_argv_pretty(&buf_payload, parent_names);
+	strbuf_addch(&buf_payload, ']');
+
+	perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,
+			 &buf_payload);
+	strbuf_release(&buf_payload);
+}
+
 static void fn_command_name_fl(const char *file, int line, const char *name,
 			       const char *hierarchy)
 {
@@ -532,6 +547,7 @@  struct tr2_tgt tr2_tgt_perf = {
 	fn_atexit,
 	fn_error_va_fl,
 	fn_command_path_fl,
+	fn_command_ancestry_fl,
 	fn_command_name_fl,
 	fn_command_mode_fl,
 	fn_alias_fl,