Re: [PATCH] fetch-pack: write effective filter to trace2

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 18, 2022, 16:18 UTC
Message-ID: <20220718161823.3363013-1-jonathantanmy@google.com>
In-Reply-To: <xmqqr12is9gp.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 20 quoted lines

> > Using two different keywords.
> >
> > So that the log only contains "filter/effective" when it was actually
> > used.  And there is no "filter/effective" event when (for whatever
> > reason) it was not in effect.
> >
> > Then the "filter/unsupported" event helps you with debugging.  Did they
> > hit a server that doesn't support filtering or did they have a typo in
> > their filter spec?
> >
> > Then don't emit a message at all for the "not requested" case.  And you
> > can use the Git version number to know how to interpret it.  There are
> > other places where we don't bother sending messages where the value is
> > a zero or empty.
> 
> Sounds alright.  We could standardize the other way, which might
> make the interpretation of individual trace entries independent of
> the context easier, though.
> 
> Thanks.

Thanks for bringing up the use case of debugging a server that we expected to support filtering but doesn't. As for whether we should not send a message when the value is empty, I can see at least one reason for not sending it - to not waste I/O and clutter the trace because of a feature that the user is not using, but fetch is I/O-intensive enough and having the empty message is useful enough (not only do we not need to know which versions have this feature, but we also can be sure that the message wasn't excluded because of some unexpected log filtering or something like that) that I think we should have the empty message. I'll put it in v2 but we can easily remove it if we decide that we don't want it.