Re: [PATCH] upload-pack: add tracing for fetches

[Robert Coup <robert@coup.net.nz>]

Hi Taylor,

Thanks for taking a look.

On Wed, 11 Oct 2023 at 22:32, Taylor Blau <me@ttaylorr.com> wrote:
>
> One of the custom patches that GitHub is carrying in its private
> fork is something similar to this that dumps information from
> upload-pack into a custom logging system specific to GitHub

I suspected as much. My initial goal is clones vs fetches, filter use, and
shallow use... since I'm changing code is there anything else that you think is
worthwhile instrumenting? I thought about casting the ints to booleans
(ie: >0 haves; >0 wants; etc) but I figured the actual numbers might come in
handy at some point, and changing them back in future would be incompatible. I
deliberately didn't add thin/ofs/etc flags: most of that stuff is on by default
now so it felt like noise.

> > + jw_object_begin(&jw, 0);
> > + {
>
> Is there a reason that we have a separate scope here? I think we may
> want to drop this as unnecessary, but it's entirely possible that I'm
> missing something here...

I just followed the patterns in test-json-writer.c - personally I think since
JSON objects/arrays have starts & ends then it's easy to read. But one person's
readable is another's coding style violation ;-)

> > + jw_object_string(&jw, "filter", list_object_filter_config_name(data->filter_options.choice));
>
> I'm pretty sure that list_object_filter_config_name() returns characters
> that are safe for JSON-encoding, and/or that jw_object_string() does any
> quoting beforehand, but worth checking nonetheless.

Yes, jw_object_string() calls append_quoted_string() which deals with escaping
and quoting.

Thanks,

Rob :)