Skip to content

trace2: move generation of 'def_param' events into code for 'cmd_name'

Jeff Hostetler via GitGitGadget git 2 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest3 patches 0 repliesNew
  2. v14 patches 4 repliesSuperseded

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3t0211: demonstrate missing 'def_param' events for certain commandst/t0211-trace2-perf.sh+231 -00 replies
2/3trace2: avoid emitting 'def_param' set more than oncet/t0211-trace2-perf.sh trace2.c+13 -10 replies
3/3trace2: emit 'def_param' set with 'cmd_name' eventgit.c t/t0211-trace2-perf.sh trace2.c+6 -90 replies

Reviews trailers per patch

1 of 3 patches has a Reviewed-by or Acked-by.

PatchJosh Steadmon
1/3
2/3
3/3R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [PATCH v2 0/3] trace2: move generation of 'def_param' events into code for 'cmd_name'

Here is version 2 of this series. The only change from V1 is to combine the last two commits as discussed.

Thanks Jeff

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

Some Git commands do not emit def_param events for interesting config and environment variable settings. Let's fix that.

Builtin commands compiled into git.c have the normal control flow and emit a cmd_name event and then def_param events for each interesting config and environment variable. However, some special "query" commands, like --exec-path, or some forms of alias expansion, emitted a cmd_name but did not emit def_param events.

Also, special commands git-remote-https is built from remote-curl.c and git-http-fetch is built from http-fetch.c and do not use the normal set up in git.c. These emitted a cmd_name but not def_param events.

To minimize the footprint of this commit, move the calls to trace2_cmd_list_config() and trace2_cmd_list_env_vars() into trace2_cmd_name() so that we always get a set of def_param events when a cmd_name event is generated.

Users can define local config settings on a repo to classify/name a repo (e.g. "project-foo" vs "personal") and use the def_param feature to label Trace2 data so that (a third-party) telemetry service does not collect data on personal repos or so that telemetry from one work repo is distinguishable from another work repo in database queries.

Jeff Hostetler (3):
t0211: demonstrate missing 'def_param' events for certain commands
trace2: avoid emitting 'def_param' set more than once
trace2: emit 'def_param' set with 'cmd_name' event

 git.c                  |   6 --
 t/t0211-trace2-perf.sh | 231 +++++++++++++++++++++++++++++++++++++++++
 trace2.c               |  15 +++
 3 files changed, 246 insertions(+), 6 deletions(-)

base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1679%2Fjeffhostetler%2Falways-emit-def-param-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1679/jeffhostetler/always-emit-def-param-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1679

Range-diff vs v1:

 1:  b378b93242a = 1:  b378b93242a t0211: demonstrate missing 'def_param' events for certain commands
 2:  65068e97597 = 2:  65068e97597 trace2: avoid emitting 'def_param' set more than once
 3:  9507184b4f1 ! 3:  178721cd4f0 trace2: emit 'def_param' set with 'cmd_name' event

@@ Commit message
the "trace2_cmd_name()" function to generate the set of 'def_param' events.

- We can later remove explicit calls to "trace2_cmd_list_config()" and
- "trace2_cmd_list_env_vars()" in git.c.
+ Remove explicit calls to "trace2_cmd_list_config()" and
+ "trace2_cmd_list_env_vars()" in git.c since they are no longer needed.

+ Reviewed-by: Josh Steadmon <[email protected]>

+ ## git.c ##
+@@ git.c: static int handle_alias(int *argcp, const char ***argv)
+ strvec_pushv(&child.args, (*argv) + 1);
+
+ trace2_cmd_alias(alias_command, child.args.v);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+ trace2_cmd_name("_run_shell_alias_");
+
+ ret = run_command(&child);
+@@ git.c: static int handle_alias(int *argcp, const char ***argv)
+ COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
+
+ trace2_cmd_alias(alias_command, new_argv);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+
+ *argv = new_argv;
+ *argcp += count - 1;
+@@ git.c: static int run_builtin(struct cmd_struct *p, int argc, const char **argv) +
+ trace_argv_printf(argv, "trace: built-in: git");
+ trace2_cmd_name(p->cmd);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+
+ validate_cache_entries(the_repository->index);
+ status = p->fn(argc, argv, prefix);
+
## t/t0211-trace2-perf.sh ##
@@ t/t0211-trace2-perf.sh: test_expect_success 'expect def_params for normal builtin command' '
# Representative query command dispatched in handle_options()
4: e8528715ebf < -: ----------- trace2: remove unneeded calls to generate 'def_param' set

Signature · 1 line
gitgitgadget