| Message ID | 5d5097b67109554e0763724633810ea616b5e2b2.1604355792.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Advertise trace2 SID in protocol capabilities | expand |
Josh Steadmon <steadmon@google.com> 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? We are not exposing the session ID anywhere but the transports, so how about calling it transport.advertiseSID, perhaps? We also may want to call that just "session ID", not "trace2 session ID" in the description. Thanks. > Signed-off-by: Josh Steadmon <steadmon@google.com> > --- > 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.
On 2020.11.03 13:42, Junio C Hamano wrote: > Josh Steadmon <steadmon@google.com> 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 <steadmon@google.com> > > --- > > 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.
Josh Steadmon <steadmon@google.com> writes: > 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. If somebody other than trace2 starts using session ID, or if we introduce a mechanism that allows a session ID assigned without enabling the rest of the trace2 machinery, such a separation may make sense at the implementation level, but until then, I do not think it is worth doing.
Hi Junio, On Thu, 5 Nov 2020, Junio C Hamano wrote: > Josh Steadmon <steadmon@google.com> writes: > > > 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. > > If somebody other than trace2 starts using session ID, or if we > introduce a mechanism that allows a session ID assigned without > enabling the rest of the trace2 machinery, such a separation may > make sense at the implementation level, but until then, I do not > think it is worth doing. Yep, the closest we came to having another user of the session ID was Matthew DeVore's `git xl` effort, but that effort seems to have gone cold (https://lore.kernel.org/git/20200207013918.GA459@comcast.net/#r is the latest message in that thread that I could find). Ciao, Dscho
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.
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. Signed-off-by: Josh Steadmon <steadmon@google.com> --- Documentation/config/trace2.txt | 4 ++++ 1 file changed, 4 insertions(+)