diff mbox series

[v6,7/7] tr2: dump names if config exist in multiple scopes

Message ID c45ead51ffc5a9176493d627da8332d35a31d87c.1657540174.git.dyroneteng@gmail.com (mailing list archive)
State Superseded
Headers show
Series trace2: dump scope when print "interesting" config | expand

Commit Message

Teng Long July 11, 2022, 12:44 p.m. UTC 
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>
---
 trace2/tr2_tgt_event.c  | 3 +++
 trace2/tr2_tgt_normal.c | 5 ++++-
 trace2/tr2_tgt_perf.c   | 9 +++++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

Comments

Ævar Arnfjörð Bjarmason July 11, 2022, 2:40 p.m. UTC | #1 
On Mon, Jul 11 2022, 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"}

This seems sensible on its face, but...

> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  trace2/tr2_tgt_event.c  | 3 +++
>  trace2/tr2_tgt_normal.c | 5 ++++-
>  trace2/tr2_tgt_perf.c   | 9 +++++++--
>  3 files changed, 14 insertions(+), 3 deletions(-)

... we really should update Documentation/technical/api-trace2.txt here too.

It does say "..." currently, so we're not lying there, but since we now
add "scope" unconditionally...
Jeff Hostetler July 11, 2022, 7:19 p.m. UTC | #2 
On 7/11/22 8:44 AM, 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>
> ---
>   trace2/tr2_tgt_event.c  | 3 +++
>   trace2/tr2_tgt_normal.c | 5 ++++-
>   trace2/tr2_tgt_perf.c   | 9 +++++++--
>   3 files changed, 14 insertions(+), 3 deletions(-)
> 
> 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,
> 

Nicely done.

Thanks for your attention on this.
Jeff
diff mbox series

Patch

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,