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

Message

Emily Shaffer July 22, 2021, 1:27 a.m. UTC

Since v5, I reshuffled some of the platform-specific compilation recipe
around per Jeff H's comments on v5. I think these are at odds with
Ævar's comments, though, so I guess let's take a look and see how
strongly we feel? :)

Otherwise I also made the logic inversion Junio suggested to make the
logic easier to follow in procinfo.c:trace2_collect_process_info.

Thanks,
 - Emily

Emily Shaffer (2):
  tr2: make process info collection platform-generic
  tr2: log parent process name

 Documentation/technical/api-trace2.txt | 14 +++++++
 Makefile                               |  4 ++
 compat/linux/procinfo.c                | 55 ++++++++++++++++++++++++++
 compat/stub/procinfo.c                 | 11 ++++++
 config.mak.uname                       |  3 ++
 t/t0210/scrub_normal.perl              |  6 +++
 t/t0211/scrub_perf.perl                |  5 +++
 t/t0212/parse_events.perl              |  5 ++-
 trace2.c                               | 13 ++++++
 trace2.h                               | 16 +++++---
 trace2/tr2_tgt.h                       |  3 ++
 trace2/tr2_tgt_event.c                 | 21 ++++++++++
 trace2/tr2_tgt_normal.c                | 19 +++++++++
 trace2/tr2_tgt_perf.c                  | 16 ++++++++
 14 files changed, 184 insertions(+), 7 deletions(-)
 create mode 100644 compat/linux/procinfo.c
 create mode 100644 compat/stub/procinfo.c

Range-diff against v5:
-:  ---------- > 1:  80084448e4 tr2: make process info collection platform-generic
1:  7a7e1ebbfa ! 2:  485f9a24f0 tr2: log parent process name
    @@ Documentation/technical/api-trace2.txt: about specific error arguments.
      	This event contains the command name for this git process
      	and the hierarchy of commands from parent git processes.
     
    - ## Makefile ##
    -@@ Makefile: ifneq ($(PROCFS_EXECUTABLE_PATH),)
    - 	BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
    - endif
    - 
    -+ifdef HAVE_PROCFS_LINUX
    -+	BASIC_CFLAGS += -DHAVE_PROCFS_LINUX
    -+	COMPAT_OBJS += compat/procinfo.o
    -+endif
    -+
    - ifdef HAVE_NS_GET_EXECUTABLE_PATH
    - 	BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
    - endif
    -
    - ## compat/procinfo.c (new) ##
    + ## compat/linux/procinfo.c (new) ##
     @@
     +#include "cache.h"
     +
    @@ compat/procinfo.c (new)
     +
     +static void get_ancestry_names(struct strvec *names)
     +{
    -+#ifdef HAVE_PROCFS_LINUX
     +	/*
     +	 * NEEDSWORK: We could gather the entire pstree into an array to match
     +	 * functionality with compat/win32/trace2_win32_process_info.c.
    @@ compat/procinfo.c (new)
     +	}
     +
     +	return;
    -+#endif
     +	/* NEEDSWORK: add non-procfs-linux implementations here */
     +}
     +
    @@ compat/procinfo.c (new)
     +
     +		get_ancestry_names(&names);
     +
    -+		if (names.nr == 0) {
    -+			strvec_clear(&names);
    -+			return;
    -+		}
    -+
    -+		trace2_cmd_ancestry(names.v);
    -+
    ++		if (names.nr)
    ++			trace2_cmd_ancestry(names.v);
     +		strvec_clear(&names);
     +	}
     +
    @@ config.mak.uname: ifeq ($(uname_S),Linux)
      	FREAD_READS_DIRECTORIES = UnfortunatelyYes
      	BASIC_CFLAGS += -DHAVE_SYSINFO
      	PROCFS_EXECUTABLE_PATH = /proc/self/exe
    -+	HAVE_PROCFS_LINUX = YesPlease
    ++	HAVE_PLATFORM_PROCINFO = YesPlease
    ++	COMPAT_OBJS += compat/linux/procinfo.o
      endif
      ifeq ($(uname_S),GNU/kFreeBSD)
      	HAVE_ALLOCA_H = YesPlease
    @@ trace2.h: void trace2_cmd_path_fl(const char *file, int line, const char *pathna
      /*
       * Emit a 'cmd_name' event with the canonical name of the command.
       * This gives post-processors a simple field to identify the command
    -@@ trace2.h: enum trace2_process_info_reason {
    - 	TRACE2_PROCESS_INFO_EXIT,
    - };
    - 
    --#if defined(GIT_WINDOWS_NATIVE)
    -+#if ( defined(GIT_WINDOWS_NATIVE) || defined(HAVE_PROCFS_LINUX) )
    - void trace2_collect_process_info(enum trace2_process_info_reason reason);
    - #else
    - #define trace2_collect_process_info(reason) \
     
      ## trace2/tr2_tgt.h ##
     @@ trace2/tr2_tgt.h: typedef void(tr2_tgt_evt_error_va_fl_t)(const char *file, int line,

Comments

Jeff Hostetler July 22, 2021, 4:59 p.m. UTC | #1

On 7/21/21 9:27 PM, Emily Shaffer wrote:
> Since v5, I reshuffled some of the platform-specific compilation recipe
> around per Jeff H's comments on v5. I think these are at odds with
> Ævar's comments, though, so I guess let's take a look and see how
> strongly we feel? :)
> 
> Otherwise I also made the logic inversion Junio suggested to make the
> logic easier to follow in procinfo.c:trace2_collect_process_info.
> 
> Thanks,
>   - Emily
...

LGTM

Thanks,
Jeff