All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Steadmon <steadmon@google.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, me@ttaylorr.com, avarab@gmail.com
Subject: [PATCH v3] connect, protocol: log negotiated protocol version
Date: Tue, 10 Aug 2021 10:20:39 -0700	[thread overview]
Message-ID: <48180cecc7a8013ec19ed8fc19842d81b356ebd7.1628615309.git.steadmon@google.com> (raw)
In-Reply-To: <f79ab95af6da3da710da2112cfcae4a042b7f7fb.1628020616.git.steadmon@google.com>

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(+)

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
-- 
2.32.0.605.g8dce9f2422-goog


  parent reply	other threads:[~2021-08-10 17:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 20:13 [PATCH] connect, protocol: log negotiated protocol version Josh Steadmon
2021-08-03 21:12 ` Taylor Blau
2021-08-04 21:37   ` Josh Steadmon
2021-08-04 21:56     ` Junio C Hamano
2021-08-04 22:17 ` [PATCH v2 0/2] " Josh Steadmon
2021-08-04 22:17   ` [PATCH v2 1/2] protocol: add protocol version formatting function Josh Steadmon
2021-08-04 23:32     ` Ævar Arnfjörð Bjarmason
2021-08-04 22:17   ` [PATCH v2 2/2] connect, protocol: log negotiated protocol version Josh Steadmon
2021-08-04 22:28     ` Eric Sunshine
2021-08-06 21:15       ` Josh Steadmon
2021-08-04 23:40     ` Ævar Arnfjörð Bjarmason
2021-08-05  1:26       ` Taylor Blau
2021-08-05  2:47         ` Ævar Arnfjörð Bjarmason
2021-08-06 21:22           ` Josh Steadmon
2021-08-10 17:20 ` Josh Steadmon [this message]
2021-08-16 18:03   ` [PATCH v3] " Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48180cecc7a8013ec19ed8fc19842d81b356ebd7.1628615309.git.steadmon@google.com \
    --to=steadmon@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.