Subject: Re: [PATCH v2 02/11] docs: new trace2.advertiseSID option

From: Josh Steadmon <[email-masked]>
Date: Thu, 5 Nov 2020 11:28:47 -0800

On 2020.11.03 13:42, Junio C Hamano wrote:
> Josh Steadmon <[email-masked]> writes:
> 
> > Document a new config option that allows users to determine whether or
> > not to advertise their trace2 session IDs to remote Git clients and
> > servers.
> 
> I do not think placeing this in the trace2 hierarchy is a good idea.
> 
> It is not like concept of "session" belongs to trace2; each
> operation we perform inherently is done on behalf of a session.
> The trace2 subsystem may have been the first to externalize the
> concept, but even after trace2 gets superseded, we would want to
> key our log records with some "session ID".  After we introduce
> an improved mechanism that is successor to trace2, we still would
> want to exchange some session ID if the advertiseSID option the
> users define in their repository today (well, maybe in 3 months
> after this series lands in a released version and widely
> deployed), no?

Yes this makes sense. Do you think it's worthwhile to move all the
session ID implementation out of trace2? Right now there are some
user-facing bits (environment variables for parent/child SID hierarchy)
that specifically mention trace2, and I believe that the repo tool is
using it to tie together logs produced by a single repo invocation.

> We are not exposing the session ID anywhere but the transports, so
> how about calling it transport.advertiseSID, perhaps?

Yeah, this sounds good to me. Will change in V3.

> We also may want to call that just "session ID", not "trace2
> session ID" in the description.
> 
> Thanks.
> 
> > Signed-off-by: Josh Steadmon <[email-masked]>
> > ---
> >  Documentation/config/trace2.txt | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/config/trace2.txt b/Documentation/config/trace2.txt
> > index 01d3afd8a8..3f2e3b4425 100644
> > --- a/Documentation/config/trace2.txt
> > +++ b/Documentation/config/trace2.txt
> > @@ -69,3 +69,7 @@ trace2.maxFiles::
> >  	write additional traces if we would exceed this many files. Instead,
> >  	write a sentinel file that will block further tracing to this
> >  	directory. Defaults to 0, which disables this check.
> > +
> > +trace2.advertiseSID::
> > +	Boolean. When true, client and server processes will advertise their
> > +	trace2 session IDs to their remote counterpart. Defaults to false.