Web lists-archives.com

[PATCH v2 0/7] trace2: load trace2 settings from system config




Here is version 2. It addresses most the V1 comments WRT the system config
changes.

It also addresses the format and uniqueness of the SID as discussed in [1].
The SID now containes: the UTC date/time, part of SHA1 of the hostname, and
the PID and is formatted to make it safe for filenames.

It also contains (a somewhat unrelated platform-specific) commit to report
total process memory usage at exit. This is helpful when looking for
problematic commands that might have scaling problems.

[1] 
https://public-inbox.org/git/51e88650-8667-df1f-13ef-4537f2e70346@xxxxxxxxxxxxxxxxx/T/#m6b4e6f2b0374d5ba88de8d0350ce6bf51b28d7da


----------------------------------------------------------------------------

Teach git to load default Trace2 settings from the system config (usually
"/etc/gitconfig"). The existing GIT_TR2_* environment variables can be used
to override the new system defaults. It also includes a little startup
refactoring.

Note: I found interactive testing of this feature to be awkward on some
platforms because of the use of prefix- or runtime-prefix-relative locations
for the system configuration. It was easy to accidentally use an officially
installed version of git to set a system config variable in the official
system config directory; and then when testing with the test version of git,
that value would not be seen because it was looking for the system config
file in a different directory.

Jeff Hostetler (7):
  trace2: refactor setting process starting time
  trace2: add absolute elapsed time to start event
  trace2: find exec-dir before trace2 initialization
  trace2: use system config for default trace2 settings
  trace2: report peak memory usage of the process
  trace2: clarify UTC datetime formatting
  trace2: make SIDs more unique

 Documentation/technical/api-trace2.txt   |  66 +++++++++---
 Makefile                                 |   1 +
 common-main.c                            |   8 +-
 compat/mingw.c                           |   2 +
 compat/win32/trace2_win32_process_info.c |  50 ++++++++-
 t/t0210-trace2-normal.sh                 |  41 +++++++-
 t/t0211-trace2-perf.sh                   |  53 ++++++++--
 t/t0212-trace2-event.sh                  |  52 ++++++++-
 trace2.c                                 |  21 +++-
 trace2.h                                 |  43 ++++++--
 trace2/tr2_cfg.c                         |   7 +-
 trace2/tr2_dst.c                         |  24 ++---
 trace2/tr2_dst.h                         |   3 +-
 trace2/tr2_sid.c                         |  39 ++++++-
 trace2/tr2_sysenv.c                      | 128 +++++++++++++++++++++++
 trace2/tr2_sysenv.h                      |  36 +++++++
 trace2/tr2_tbuf.c                        |  20 +++-
 trace2/tr2_tbuf.h                        |   5 +-
 trace2/tr2_tgt.h                         |   1 +
 trace2/tr2_tgt_event.c                   |  53 +++++-----
 trace2/tr2_tgt_normal.c                  |  19 ++--
 trace2/tr2_tgt_perf.c                    |  23 ++--
 trace2/tr2_tls.c                         |  38 ++++---
 trace2/tr2_tls.h                         |   8 +-
 24 files changed, 604 insertions(+), 137 deletions(-)
 create mode 100644 trace2/tr2_sysenv.c
 create mode 100644 trace2/tr2_sysenv.h


base-commit: 041f5ea1cf987a4068ef5f39ba0a09be85952064
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-169%2Fjeffhostetler%2Fcore-tr2-startup-and-sysenv-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-169/jeffhostetler/core-tr2-startup-and-sysenv-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/169

Range-diff vs v1:

 1:  7f1a347322 = 1:  7f1a347322 trace2: refactor setting process starting time
 2:  c15f783c26 = 2:  c15f783c26 trace2: add absolute elapsed time to start event
 3:  d4023acde7 = 3:  d4023acde7 trace2: find exec-dir before trace2 initialization
 4:  7e0d4e20fb ! 4:  d048f3ffb8 trace2: use system config for default trace2 settings
     @@ -332,11 +332,16 @@
        *
      - * The environment variable "GIT_TR2_CONFIG_PARAMS" can be set to a
      - * list of patterns considered important.  For example:
     -+ * Use the TR2_SYSENV_CFG_PARAM setting to register a list of patterns
     -+ * configured important.  For example:
     -  *
     -  *    GIT_TR2_CONFIG_PARAMS="core.*,remote.*.url"
     +- *
     +- *    GIT_TR2_CONFIG_PARAMS="core.*,remote.*.url"
     ++ * Use the TR2_SYSENV_CFG_PARAM setting to register a comma-separated
     ++ * list of patterns configured important.  For example:
     ++ *     git config --system trace2.configParams 'core.*,remote.*.url'
     ++ * or:
     ++ *     GIT_TR2_CONFIG_PARAMS=core.*,remote.*.url"
        *
     +  * Note: this routine does a read-only iteration on the config data
     +  * (using read_early_config()), so it must not be called until enough
      
       diff --git a/trace2/tr2_cfg.c b/trace2/tr2_cfg.c
       --- a/trace2/tr2_cfg.c
     @@ -527,6 +532,9 @@
      +{
      +	int k;
      +
     ++	if (!starts_with(key, "trace2."))
     ++		return 0;
     ++
      +	for (k = 0; k < ARRAY_SIZE(tr2_sysenv_settings); k++) {
      +		if (!strcmp(key, tr2_sysenv_settings[k].git_config_name)) {
      +			free(tr2_sysenv_settings[k].value);
     @@ -673,7 +681,8 @@
      + * region details in the event target.
        */
      -#define TR2_ENVVAR_EVENT_NESTING "GIT_TR2_EVENT_NESTING"
     - static int tr2env_event_nesting_wanted = 2;
     +-static int tr2env_event_nesting_wanted = 2;
     ++static int tr2env_event_max_nesting_levels = 2;
       
       /*
      - * Set this environment variable to true to omit the <time>, <file>, and
     @@ -681,12 +690,14 @@
        * <line> fields from most events.
        */
      -#define TR2_ENVVAR_EVENT_BRIEF "GIT_TR2_EVENT_BRIEF"
     - static int tr2env_event_brief;
     +-static int tr2env_event_brief;
     ++static int tr2env_event_be_brief;
       
       static int fn_init(void)
     -@@
     + {
       	int want = tr2_dst_trace_want(&tr2dst_event);
     - 	int want_nesting;
     +-	int want_nesting;
     ++	int max_nesting;
       	int want_brief;
      -	char *nesting;
      -	char *brief;
     @@ -697,15 +708,74 @@
       		return want;
       
      -	nesting = getenv(TR2_ENVVAR_EVENT_NESTING);
     +-	if (nesting && ((want_nesting = atoi(nesting)) > 0))
     +-		tr2env_event_nesting_wanted = want_nesting;
      +	nesting = tr2_sysenv_get(TR2_SYSENV_EVENT_NESTING);
     - 	if (nesting && ((want_nesting = atoi(nesting)) > 0))
     - 		tr2env_event_nesting_wanted = want_nesting;
     ++	if (nesting && *nesting && ((max_nesting = atoi(nesting)) > 0))
     ++		tr2env_event_max_nesting_levels = max_nesting;
       
      -	brief = getenv(TR2_ENVVAR_EVENT_BRIEF);
     +-	if (brief && ((want_brief = atoi(brief)) > 0))
     +-		tr2env_event_brief = want_brief;
      +	brief = tr2_sysenv_get(TR2_SYSENV_EVENT_BRIEF);
     - 	if (brief && ((want_brief = atoi(brief)) > 0))
     - 		tr2env_event_brief = want_brief;
     ++	if (brief && *brief &&
     ++	    ((want_brief = git_parse_maybe_bool(brief)) != -1))
     ++		tr2env_event_be_brief = want_brief;
       
     + 	return want;
     + }
     +@@
     + 	/*
     + 	 * In brief mode, only emit <time> on these 2 event types.
     + 	 */
     +-	if (!tr2env_event_brief || !strcmp(event_name, "version") ||
     ++	if (!tr2env_event_be_brief || !strcmp(event_name, "version") ||
     + 	    !strcmp(event_name, "atexit")) {
     + 		tr2_tbuf_utc_time(&tb_now);
     + 		jw_object_string(jw, "time", tb_now.buf);
     + 	}
     + 
     +-	if (!tr2env_event_brief && file && *file) {
     ++	if (!tr2env_event_be_brief && file && *file) {
     + 		jw_object_string(jw, "file", file);
     + 		jw_object_intmax(jw, "line", line);
     + 	}
     +@@
     + {
     + 	const char *event_name = "region_enter";
     + 	struct tr2tls_thread_ctx *ctx = tr2tls_get_self();
     +-	if (ctx->nr_open_regions <= tr2env_event_nesting_wanted) {
     ++	if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) {
     + 		struct json_writer jw = JSON_WRITER_INIT;
     + 
     + 		jw_object_begin(&jw, 0);
     +@@
     + {
     + 	const char *event_name = "region_leave";
     + 	struct tr2tls_thread_ctx *ctx = tr2tls_get_self();
     +-	if (ctx->nr_open_regions <= tr2env_event_nesting_wanted) {
     ++	if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) {
     + 		struct json_writer jw = JSON_WRITER_INIT;
     + 		double t_rel = (double)us_elapsed_region / 1000000.0;
     + 
     +@@
     + {
     + 	const char *event_name = "data";
     + 	struct tr2tls_thread_ctx *ctx = tr2tls_get_self();
     +-	if (ctx->nr_open_regions <= tr2env_event_nesting_wanted) {
     ++	if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) {
     + 		struct json_writer jw = JSON_WRITER_INIT;
     + 		double t_abs = (double)us_elapsed_absolute / 1000000.0;
     + 		double t_rel = (double)us_elapsed_region / 1000000.0;
     +@@
     + {
     + 	const char *event_name = "data_json";
     + 	struct tr2tls_thread_ctx *ctx = tr2tls_get_self();
     +-	if (ctx->nr_open_regions <= tr2env_event_nesting_wanted) {
     ++	if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) {
     + 		struct json_writer jw = JSON_WRITER_INIT;
     + 		double t_abs = (double)us_elapsed_absolute / 1000000.0;
     + 		double t_rel = (double)us_elapsed_region / 1000000.0;
      
       diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
       --- a/trace2/tr2_tgt_normal.c
     @@ -730,9 +800,11 @@
        * Unit tests may want to use this to help with testing.
        */
      -#define TR2_ENVVAR_NORMAL_BRIEF "GIT_TR2_BRIEF"
     - static int tr2env_normal_brief;
     +-static int tr2env_normal_brief;
     ++static int tr2env_normal_be_brief;
       
       #define TR2FMT_NORMAL_FL_WIDTH (50)
     + 
      @@
       {
       	int want = tr2_dst_trace_want(&tr2dst_normal);
     @@ -747,7 +819,20 @@
      +	brief = tr2_sysenv_get(TR2_SYSENV_NORMAL_BRIEF);
       	if (brief && *brief &&
       	    ((want_brief = git_parse_maybe_bool(brief)) != -1))
     - 		tr2env_normal_brief = want_brief;
     +-		tr2env_normal_brief = want_brief;
     ++		tr2env_normal_be_brief = want_brief;
     + 
     + 	return want;
     + }
     +@@
     + {
     + 	strbuf_setlen(buf, 0);
     + 
     +-	if (!tr2env_normal_brief) {
     ++	if (!tr2env_normal_be_brief) {
     + 		struct tr2_tbuf tb_now;
     + 
     + 		tr2_tbuf_local_time(&tb_now);
      
       diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c
       --- a/trace2/tr2_tgt_perf.c
     @@ -772,9 +857,11 @@
        * Unit tests may want to use this to help with testing.
        */
      -#define TR2_ENVVAR_PERF_BRIEF "GIT_TR2_PERF_BRIEF"
     - static int tr2env_perf_brief;
     +-static int tr2env_perf_brief;
     ++static int tr2env_perf_be_brief;
       
       #define TR2FMT_PERF_FL_WIDTH (50)
     + #define TR2FMT_PERF_MAX_EVENT_NAME (12)
      @@
       {
       	int want = tr2_dst_trace_want(&tr2dst_perf);
     @@ -791,4 +878,17 @@
      +	brief = tr2_sysenv_get(TR2_SYSENV_PERF_BRIEF);
       	if (brief && *brief &&
       	    ((want_brief = git_parse_maybe_bool(brief)) != -1))
     - 		tr2env_perf_brief = want_brief;
     +-		tr2env_perf_brief = want_brief;
     ++		tr2env_perf_be_brief = want_brief;
     + 
     + 	return want;
     + }
     +@@
     + 
     + 	strbuf_setlen(buf, 0);
     + 
     +-	if (!tr2env_perf_brief) {
     ++	if (!tr2env_perf_be_brief) {
     + 		struct tr2_tbuf tb_now;
     + 
     + 		tr2_tbuf_local_time(&tb_now);
 -:  ---------- > 5:  86d8fe28cf trace2: report peak memory usage of the process
 -:  ---------- > 6:  ae11cc4186 trace2: clarify UTC datetime formatting
 -:  ---------- > 7:  4352952677 trace2: make SIDs more unique

-- 
gitgitgadget