On Mon, Jun 28, 2021 at 09:19:23PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Now that we've factored out "call_{advertise,command}()" in a
> preceding commit it becomes easy to trace all these callbacks with
> trace2. Let's do that. As the tests demonstrate there's no v2 push
> protocol, which the tests assert.
Seems reasonable. I haven't ever wanted these myself, but it seems like
a natural spot to mention when debugging server-side actions (especially
because we may get multiple rounds of "fetch" for a single upload-pack
invocation).
> diff --git a/serve.c b/serve.c
> index 85cd3eab26e..6dbd05248b9 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -111,14 +111,34 @@ static struct protocol_capability capabilities[] = {
> static int call_advertise(struct protocol_capability *command,
> struct repository *r, struct strbuf *value)
> {
> - return command->advertise(r, value);
> + int ret;
> + struct strbuf sb = STRBUF_INIT;
> + const char *msg;
> +
> + strbuf_addf(&sb, "advertise/%s", command->name);
> + trace2_region_enter("serve", sb.buf, r);
> + ret = command->advertise(r, value);
> + msg = ret ? "advertised" : "hidden";
> + trace2_region_leave_printf("serve", sb.buf, r, "%s", msg);
> + strbuf_release(&sb);
We'll do these allocations even if trace2 isn't enabled. I guess that's
probably not that big a deal in practice. I think:
if (trace2_is_enabled())
strbuf_addf(&sb, "advertise/%s", command->name);
would work (everything else is cheap and handles the unallocated state
fine), but it might not be worth the readability hit (and it's probably
premature optimization anyway).
-Peff
