Subject: Re: [PATCH v2 06/11] serve: advertise trace2 SID in v2 capabilities

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Nov 2020 13:11:46 -0800

Josh Steadmon <[email-masked]> writes:

> When trace2 is enabled and trace2.advertiseSID is true, advertise the
> server's trace2 session ID for all protocol v2 connections via the new
> trace2-sid capability.
>
> Signed-off-by: Josh Steadmon <[email-masked]>
> ---
>  serve.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/serve.c b/serve.c
> index f6341206c4..b40d7aad34 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -8,6 +8,8 @@
>  #include "serve.h"
>  #include "upload-pack.h"
>  
> +static int advertise_trace2_sid;
> +
>  static int always_advertise(struct repository *r,
>  			    struct strbuf *value)
>  {
> @@ -30,6 +32,15 @@ static int object_format_advertise(struct repository *r,
>  	return 1;
>  }
>  
> +static int trace2_advertise(struct repository *r, struct strbuf *value)
> +{
> +	if (!advertise_trace2_sid || !trace2_is_enabled())
> +		return 0;
> +	if (value)
> +		strbuf_addstr(value, trace2_session_id());
> +	return 1;
> +}

To both 05/11 and 06/11, the same "if the user instructs us to
advertise, shouldn't we arrange the thing we were told to advertise
to be automatically available by at least allocating a session ID,
if not enabling the entire tracing?" applies.

>  struct protocol_capability {
>  	/*
>  	 * The name of the capability.  The server uses this name when
> @@ -66,6 +77,7 @@ static struct protocol_capability capabilities[] = {
>  	{ "fetch", upload_pack_advertise, upload_pack_v2 },
>  	{ "server-option", always_advertise, NULL },
>  	{ "object-format", object_format_advertise, NULL },
> +	{ "trace2-sid", trace2_advertise, NULL },
>  };
>  
>  static void advertise_capabilities(void)
> @@ -261,6 +273,8 @@ static int process_request(void)
>  /* Main serve loop for protocol version 2 */
>  void serve(struct serve_options *options)
>  {
> +	git_config_get_bool("trace2.advertisesid", &advertise_trace2_sid);
> +
>  	if (options->advertise_capabilities || !options->stateless_rpc) {
>  		/* serve by default supports v2 */
>  		packet_write_fmt(1, "version 2\n");