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
