Re: [PATCH v2 01/11] docs: new capability to advertise trace2 SIDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/3/20 4:33 PM, Junio C Hamano wrote:
Josh Steadmon <steadmon@xxxxxxxxxx> writes:

+trace2-sid=<session-id>
+-----------------------
+
+If trace2 tracing is enabled on the server, it may advertise its session ID via
+this capability. The client may choose to log the server's session ID in its
+trace logs, and advertise its own session ID back to the server for it to log
+as well. This allows for easier debugging of remote sessions when both client
+and server logs are available.
diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
index e597b74da3..a5b9ef04f6 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -492,3 +492,12 @@ form `object-format=X`) to notify the client that the server is able to deal
  with objects using hash algorithm X.  If not specified, the server is assumed to
  only handle SHA-1.  If the client would like to use a hash algorithm other than
  SHA-1, it should specify its object-format string.
+
+trace2-sid=<session-id>
+~~~~~~~~~~~~~~~~~~~~~~~
+
+If trace2 tracing is enabled on the server, it may advertise its session ID via
+this capability. The client may choose to log the server's session ID in its
+trace logs, and advertise its own session ID back to the server for it to log
+as well. This allows for easier debugging of remote sessions when both client
+and server logs are available.

Have we documented what a session-id should look like anywhere in
the documentation?  This document is to help third-party to write
implementations of the protocol, but the above description leaves
things "implementation defined" a bit too much, I am afraid.

For example, as this must fit on a single pkt-line as an advertised
capability, there would be some length limit.  Are there other
inherent limitations due to our protocol?  Are there some artificial
limitations that we may want to impose to make it easier to harden
implementations against common mistakes?  For example are bytes in
<session-id> allowed to contain LF, CR, NUL, etc.?


The computed part of trace2 SIDs are relatively safe (both for length
and funky characters).  And funky characters are protected by JSON
encoding rules when writing to the GIT_TRACE2_EVENT target.

And I think the computed part would be safe in this context.
I've not seen commands that spawn more than about 6 levels of
child processes, and those would be fine here.

However, the opportunity to introduce a prefix as I suggested earlier
does offer the opportunity to introduce funky chars that would not
have any protection, so you may want to c-quote the string when
inserting it into the wire protocol.

    $ GIT_TRACE2_EVENT=1 GIT_TRACE2_PARENT_SID=hello git version
{"event":"version","sid":"hello/20201030T154143.660608Z-H86606a97- P00001d30",...}
    ...

(Allowing the user to insert a prefix like that has been useful for
tracking control/experiment testing, so I wouldn't want to disable
it.)

Jeff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux