| Message ID | a01ae8478d3a8545241c5b064b6d369a330ee59f.1658159746.git.dyroneteng@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trace2: dump scope when print "interesting" config | expand |
On 7/18/22 12:46 PM, Teng Long wrote: > When we specify GIT_TRACE2_CONFIG_PARAMS or trace2.configparams, > trace2 will prints "interesting" config values to log. Sometimes, > when a config set in multiple scope files, the following output > looks like (the irrelevant fields are omitted here as "..."): > > ...| def_param | ... | core.multipackindex:false > ...| def_param | ... | core.multipackindex:false > ...| def_param | ... | core.multipackindex:false > > As the log shows, even each config in different scope is dumped, but > we don't know which scope it comes from. Therefore, it's better to > add the scope names as well to make them be more recognizable. For > example, when execute: > > $ GIT_TRACE2_PERF=1 \ > > GIT_TRACE2_CONFIG_PARAMS=core.multipackIndex \ > > git rev-list --test-bitmap HEAD" > > The following is the ouput (the irrelevant fields are omitted here > as "..."): > > Format normal: > ... git.c:461 ... def_param scope:system core.multipackindex=false > ... git.c:461 ... def_param scope:global core.multipackindex=false > ... git.c:461 ... def_param scope:local core.multipackindex=false > > Format perf: > > ... | def_param | ... | scope:system | core.multipackindex:false > ... | def_param | ... | scope:global | core.multipackindex:false > ... | def_param | ... | scope:local | core.multipackindex:false > > Format event: > > {"event":"def_param", ... ,"scope":"system","param":"core.multipackindex","value":"false"} > {"event":"def_param", ... ,"scope":"global","param":"core.multipackindex","value":"false"} > {"event":"def_param", ... ,"scope":"local","param":"core.multipackindex","value":"false"} > > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > Documentation/technical/api-trace2.txt | 45 ++++++++++++++++++++++++++ > trace2/tr2_tgt_event.c | 3 ++ > trace2/tr2_tgt_normal.c | 5 ++- > trace2/tr2_tgt_perf.c | 9 ++++-- > 4 files changed, 59 insertions(+), 3 deletions(-) > > diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt > index bb13ca3db8..48205a5ac5 100644 > --- a/Documentation/technical/api-trace2.txt > +++ b/Documentation/technical/api-trace2.txt > @@ -1207,6 +1207,51 @@ at offset 508. > This example also shows that thread names are assigned in a racy manner > as each thread starts and allocates TLS storage. > > +Print Configs:: > + > + Dump "interesting" config values to trace2 log. > ++ > +The environment variable `GIT_TRACE2_CONFIG_PARAMS` and configuration > +`trace2.configparams` can be used to output config values which you care > +about. For example, assume that we want to config different `color.ui` > +values in multiple scopes, such as: > ++ > +---------------- > +$ git config --system color.ui never > +$ git config --global color.ui always > +$ git config --local color.ui auto > +$ git config --list --show-scope | grep 'color.ui' > +system color.ui=never > +global color.ui=always > +local color.ui=auto > +---------------- > ++ > +Then, mark the config `color.ui` as "interesting" config with > +`GIT_TRACE2_CONFIG_PARAMS`: > ++ > +---------------- > +$ export GIT_TRACE2_PERF_BRIEF=1 > +$ export GIT_TRACE2_PERF=~/log.perf > +$ export GIT_TRACE2_CONFIG_PARAMS=color.ui > +$ git version > +... > +$ cat ~/log.perf > +d0 | main | version | | | | | ... > +d0 | main | start | | 0.000284 | | | /opt/git/master/bin/git version > +d0 | main | cmd_ancestry | | | | | ancestry:[bash sshd sshd sshd systemd] > +d0 | main | cmd_name | | | | | version (version) > +d0 | main | exit | | 0.000419 | | | code:0 > +d0 | main | atexit | | 0.000426 | | | code:0 > +d0 | main | version | | | | | ... > +d0 | main | start | | 0.000275 | | | /opt/git/master/bin/git version > +d0 | main | cmd_ancestry | | | | | ancestry:[bash sshd sshd sshd systemd] > +d0 | main | cmd_name | | | | | version (version) > +d0 | main | def_param | | | | | color.ui:never > +d0 | main | def_param | | | | | color.ui:always > +d0 | main | def_param | | | | | color.ui:auto shouldn't this example in the docs have the scope value in the next to last column as you noted in the cover letter ? > +d0 | main | exit | | 0.000543 | | | code:0 > +d0 | main | atexit | | 0.000549 | | | code:0 > +---------------- > == Future Work ... Otherwise, this all LGTM. Thanks Jeff
Jeff Hostetler <git@jeffhostetler.com> writes: > shouldn't this example in the docs have the scope value in the > next to last column as you noted in the cover letter ? oops, sorry for that. I will send a quick fix today. Thanks.
Teng Long <dyroneteng@gmail.com> writes:
> Subject: Re: [PATCH v7 7/7] tr2: dump names if config exist in multiple scopes
This make it sound as if we do not show the scope if a configuration
variable appears only in one scope, but I do not see how the updated
code achieves that. Instead, it seems that it shows the scope
unconditionally in addition to the key-value pair.
If that is the case, a retitle is in order. Also this should
probably be a separate topic, unrelated from the other 6 patches.
Junio C Hamano <gitster@pobox.com> writes: > This make it sound as if we do not show the scope if a configuration > variable appears only in one scope, but I do not see how the updated > code achieves that. Instead, it seems that it shows the scope > unconditionally in addition to the key-value pair. Yes. There is a problem with this statement. I will fix it, maybe like: tr2: print corresponding scope with config key-value > If that is the case, a retitle is in order. Also this should > probably be a separate topic, unrelated from the other 6 patches. That's the case. I will send the [0-6] as the v8, and separate [7/7] as a new patch series. Thanks.
diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index bb13ca3db8..48205a5ac5 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -1207,6 +1207,51 @@ at offset 508. This example also shows that thread names are assigned in a racy manner as each thread starts and allocates TLS storage. +Print Configs:: + + Dump "interesting" config values to trace2 log. ++ +The environment variable `GIT_TRACE2_CONFIG_PARAMS` and configuration +`trace2.configparams` can be used to output config values which you care +about. For example, assume that we want to config different `color.ui` +values in multiple scopes, such as: ++ +---------------- +$ git config --system color.ui never +$ git config --global color.ui always +$ git config --local color.ui auto +$ git config --list --show-scope | grep 'color.ui' +system color.ui=never +global color.ui=always +local color.ui=auto +---------------- ++ +Then, mark the config `color.ui` as "interesting" config with +`GIT_TRACE2_CONFIG_PARAMS`: ++ +---------------- +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf +$ export GIT_TRACE2_CONFIG_PARAMS=color.ui +$ git version +... +$ cat ~/log.perf +d0 | main | version | | | | | ... +d0 | main | start | | 0.000284 | | | /opt/git/master/bin/git version +d0 | main | cmd_ancestry | | | | | ancestry:[bash sshd sshd sshd systemd] +d0 | main | cmd_name | | | | | version (version) +d0 | main | exit | | 0.000419 | | | code:0 +d0 | main | atexit | | 0.000426 | | | code:0 +d0 | main | version | | | | | ... +d0 | main | start | | 0.000275 | | | /opt/git/master/bin/git version +d0 | main | cmd_ancestry | | | | | ancestry:[bash sshd sshd sshd systemd] +d0 | main | cmd_name | | | | | version (version) +d0 | main | def_param | | | | | color.ui:never +d0 | main | def_param | | | | | color.ui:always +d0 | main | def_param | | | | | color.ui:auto +d0 | main | exit | | 0.000543 | | | code:0 +d0 | main | atexit | | 0.000549 | | | code:0 +---------------- == Future Work === Relationship to the Existing Trace Api (api-trace.txt) diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index c5c8cfbbaa..37a3163be1 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -479,9 +479,12 @@ static void fn_param_fl(const char *file, int line, const char *param, { const char *event_name = "def_param"; struct json_writer jw = JSON_WRITER_INIT; + enum config_scope scope = current_config_scope(); + const char *scope_name = config_scope_name(scope); jw_object_begin(&jw, 0); event_fmt_prepare(event_name, file, line, NULL, &jw); + jw_object_string(&jw, "scope", scope_name); jw_object_string(&jw, "param", param); jw_object_string(&jw, "value", value); jw_end(&jw); diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index c42fbade7f..69f8033077 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -298,8 +298,11 @@ static void fn_param_fl(const char *file, int line, const char *param, const char *value) { struct strbuf buf_payload = STRBUF_INIT; + enum config_scope scope = current_config_scope(); + const char *scope_name = config_scope_name(scope); - strbuf_addf(&buf_payload, "def_param %s=%s", param, value); + strbuf_addf(&buf_payload, "def_param scope:%s %s=%s", scope_name, param, + value); normal_io_write_fl(file, line, &buf_payload); strbuf_release(&buf_payload); } diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index a1eff8bea3..8cb792488c 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -441,12 +441,17 @@ static void fn_param_fl(const char *file, int line, const char *param, { const char *event_name = "def_param"; struct strbuf buf_payload = STRBUF_INIT; + struct strbuf scope_payload = STRBUF_INIT; + enum config_scope scope = current_config_scope(); + const char *scope_name = config_scope_name(scope); strbuf_addf(&buf_payload, "%s:%s", param, value); + strbuf_addf(&scope_payload, "%s:%s", "scope", scope_name); - perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL, - &buf_payload); + perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, + scope_payload.buf, &buf_payload); strbuf_release(&buf_payload); + strbuf_release(&scope_payload); } static void fn_repo_fl(const char *file, int line,
When we specify GIT_TRACE2_CONFIG_PARAMS or trace2.configparams, trace2 will prints "interesting" config values to log. Sometimes, when a config set in multiple scope files, the following output looks like (the irrelevant fields are omitted here as "..."): ...| def_param | ... | core.multipackindex:false ...| def_param | ... | core.multipackindex:false ...| def_param | ... | core.multipackindex:false As the log shows, even each config in different scope is dumped, but we don't know which scope it comes from. Therefore, it's better to add the scope names as well to make them be more recognizable. For example, when execute: $ GIT_TRACE2_PERF=1 \ > GIT_TRACE2_CONFIG_PARAMS=core.multipackIndex \ > git rev-list --test-bitmap HEAD" The following is the ouput (the irrelevant fields are omitted here as "..."): Format normal: ... git.c:461 ... def_param scope:system core.multipackindex=false ... git.c:461 ... def_param scope:global core.multipackindex=false ... git.c:461 ... def_param scope:local core.multipackindex=false Format perf: ... | def_param | ... | scope:system | core.multipackindex:false ... | def_param | ... | scope:global | core.multipackindex:false ... | def_param | ... | scope:local | core.multipackindex:false Format event: {"event":"def_param", ... ,"scope":"system","param":"core.multipackindex","value":"false"} {"event":"def_param", ... ,"scope":"global","param":"core.multipackindex","value":"false"} {"event":"def_param", ... ,"scope":"local","param":"core.multipackindex","value":"false"} Signed-off-by: Teng Long <dyroneteng@gmail.com> --- Documentation/technical/api-trace2.txt | 45 ++++++++++++++++++++++++++ trace2/tr2_tgt_event.c | 3 ++ trace2/tr2_tgt_normal.c | 5 ++- trace2/tr2_tgt_perf.c | 9 ++++-- 4 files changed, 59 insertions(+), 3 deletions(-)