| Message ID | 48180cecc7a8013ec19ed8fc19842d81b356ebd7.1628615309.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 626beebdf85a6ea5561dd8a0c912c9e9fb5622b4 |
| Headers | show |
| Series | [v3] connect, protocol: log negotiated protocol version | expand |
On Tue, Aug 10, 2021 at 10:20:39AM -0700, Josh Steadmon wrote: > It is useful for performance monitoring and debugging purposes to know > the wire protocol used for remote operations. This may differ from the > version set in local configuration due to differences in version and/or > configuration between the server and the client. Therefore, log the > negotiated wire protocol version via trace2, for both clients and > servers. > > Signed-off-by: Josh Steadmon <steadmon@google.com> > --- > > Changes in V3: > * remove unnecessary `test` calls in the new t5705 test cases > * remove the wire protocol version formatting function > * log the wire protocol version enum value directly, as in V0 of this > series > * avoid logging the protocol version if negotiation fails (i.e., if we > end up with "protocol_unknown_version") Nicely done. This version looks ready to be picked up by my eyes. Reviewed-by: Taylor Blau <me@ttaylorr.comg> Thanks, Taylor
diff --git a/connect.c b/connect.c index 70b13389ba..aff13a270e 100644 --- a/connect.c +++ b/connect.c @@ -164,6 +164,8 @@ enum protocol_version discover_version(struct packet_reader *reader) BUG("unknown protocol version"); } + trace2_data_intmax("transfer", NULL, "negotiated-version", version); + return version; } diff --git a/protocol.c b/protocol.c index 052d7edbb9..c53f7df5be 100644 --- a/protocol.c +++ b/protocol.c @@ -73,6 +73,8 @@ enum protocol_version determine_protocol_version_server(void) string_list_clear(&list, 0); } + trace2_data_intmax("transfer", NULL, "negotiated-version", version); + return version; } diff --git a/t/t5705-session-id-in-capabilities.sh b/t/t5705-session-id-in-capabilities.sh index f1d189d5bc..eb8c79aafd 100755 --- a/t/t5705-session-id-in-capabilities.sh +++ b/t/t5705-session-id-in-capabilities.sh @@ -73,6 +73,17 @@ do grep \"key\":\"server-sid\" tr2-client-events && grep \"key\":\"client-sid\" tr2-server-events ' + + test_expect_success "client & server log negotiated version (v${PROTO})" ' + test_when_finished "rm -rf local tr2-client-events tr2-server-events" && + cp -r "$LOCAL_PRISTINE" local && + GIT_TRACE2_EVENT="$(pwd)/tr2-client-events" \ + git -c protocol.version=$PROTO -C local fetch \ + --upload-pack "GIT_TRACE2_EVENT=\"$(pwd)/tr2-server-events\" git-upload-pack" \ + origin && + grep \"key\":\"negotiated-version\",\"value\":\"$PROTO\" tr2-client-events && + grep \"key\":\"negotiated-version\",\"value\":\"$PROTO\" tr2-server-events + ' done test_done
It is useful for performance monitoring and debugging purposes to know the wire protocol used for remote operations. This may differ from the version set in local configuration due to differences in version and/or configuration between the server and the client. Therefore, log the negotiated wire protocol version via trace2, for both clients and servers. Signed-off-by: Josh Steadmon <steadmon@google.com> --- Changes in V3: * remove unnecessary `test` calls in the new t5705 test cases * remove the wire protocol version formatting function * log the wire protocol version enum value directly, as in V0 of this series * avoid logging the protocol version if negotiation fails (i.e., if we end up with "protocol_unknown_version") Changes in V2: * adds a format_protocol_version() helper function * adds tests * bug fix: actually log on the server side, rather than twice on the client * moves the trace statement closer to the actual version negotiation code connect.c | 2 ++ protocol.c | 2 ++ t/t5705-session-id-in-capabilities.sh | 11 +++++++++++ 3 files changed, 15 insertions(+)