Re: [PATCH] tr2: log parent process name

From: Emily Shaffer <emilyshaffer@google.com>
To: git@vger.kernel.org
Subject: Re: [PATCH] tr2: log parent process name
Date: Fri, 7 May 2021 10:09:54 -0700	[thread overview]
Message-ID: <YJV0YibkFYUe2UcA@google.com> (raw)
In-Reply-To: <20210507002908.1495061-1-emilyshaffer@google.com>

On Thu, May 06, 2021 at 05:29:08PM -0700, Emily Shaffer wrote:
> 
> It can be useful to tell who invoked Git - was it invoked manually by a
> user via CLI or script? By an IDE? Knowing where the Git invocation came
> from can help with debugging to isolate where the problem came from.
> 
> 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 regardless of platform.
> 
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> We briefly discussed hiding this behind a config, internally. However, I
> wanted to include the parent name alongside the cmd_start event, which
> happens very early (maybe before config gathering?).
> 
> Maybe it's better to log the parent_name as its own event, since it
> shouldn't change over the lifetime of the process?
> 
> procfs is very non-portable, though - I think this won't even work on
> MacOS. So I'm curious if anybody has better suggestions for how to do
> this.

I wrote this and then I wrote nonportable tests anyways, bah. Working on
a fix now - the tests break for MacOS and Windows.

The parent_name needs to be set conditionally below.

> diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh
> index 1529155cf0..3a2a8a5b5f 100755
> --- a/t/t0212-trace2-event.sh
> +++ b/t/t0212-trace2-event.sh
> @@ -61,6 +61,7 @@ test_expect_success JSON_PP 'event stream, error event' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  }
>  	|};
> @@ -115,6 +116,7 @@ test_expect_success JSON_PP 'event stream, return code 0' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  },
>  	|  "_SID0_/_SID1_":{
> @@ -143,6 +145,7 @@ test_expect_success JSON_PP 'event stream, return code 0' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2/trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"test-tool",
>  	|    "version":"$V"
>  	|  },
>  	|  "_SID0_/_SID1_/_SID2_":{
> @@ -155,6 +158,7 @@ test_expect_success JSON_PP 'event stream, return code 0' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2/trace2/trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"$TEST_DIRECTORY/../t/helper//test-tool",
>  	|    "version":"$V"
>  	|  }
>  	|};
> @@ -192,6 +196,7 @@ test_expect_success JSON_PP 'event stream, list config' '
>  	|        "value":"hello world"
>  	|      }
>  	|    ],
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  }
>  	|};
> @@ -229,6 +234,7 @@ test_expect_success JSON_PP 'event stream, list env vars' '
>  	|        "value":"hello world"
>  	|      }
>  	|    ],
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  }
>  	|};
> @@ -263,6 +269,7 @@ test_expect_success JSON_PP 'basic trace2_data' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  }
>  	|};
> @@ -295,6 +302,7 @@ test_expect_success JSON_PP 'using global config, event stream, error event' '
>  	|    "exit_code":0,
>  	|    "hierarchy":"trace2",
>  	|    "name":"trace2",
> +	|    "parent_name":"/bin/sh",
>  	|    "version":"$V"
>  	|  }
>  	|};

 - Emily