Skip to content

Commit e586f99

Browse files
authored
gvfs-helper: emit X-Session-Id headers for requests (#862)
A common problem when tracking GVFS Protocol queries is that we don't have a way to connect client and server interactions. This is especially true in the typical case where a cache server deployment is hidden behind a load balancer. We can't even determine which cache server was used for certain requests! Add some client-identifying data to the HTTP queries using the X-Session-Id header. This will by default identify the helper process using its SID. If configured via the new gvfs.sessionKey config, it will prefix this SID with another config value. For example, Office monorepo users have an 'otel.trace2.id' config value that is a pseudonymous identifier. This allows telemetry readers to group requests by enlistment without knowing the user's identity at all. Users could opt-in to provide this identifier for investigations around their long-term performance or issues. This change makes it possible to extend this to cache server interactions. * [X] This change only applies to interactions with Azure DevOps and the GVFS Protocol.
2 parents fb711f2 + efe07e3 commit e586f99

4 files changed

Lines changed: 125 additions & 0 deletions

File tree

Documentation/config/gvfs.adoc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,11 @@ gvfs.fallback::
2626
If set to `false`, then never fallback to the origin server when the cache
2727
server fails to connect. This will alert users to failures with the cache
2828
server, but avoid causing throttling on the origin server.
29+
30+
gvfs.sessionKey::
31+
If set to a string, then the value is a config key name that will be used
32+
to set a prefix in the `X-Session-Id` header sent to the server for all
33+
GVFS Protocol calls. This allows engineering systems to improve support
34+
across client and server behavior, including any End User Pseodynomous
35+
Identifiers (EUPI) that may be configured. The `X-Session-Id` will
36+
include the SID of the current process in either case.

gvfs-helper.c

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@
251251
#include "abspath.h"
252252
#include "progress.h"
253253
#include "trace2.h"
254+
#include "trace2/tr2_sid.h"
254255
#include "wrapper.h"
255256
#include "packfile.h"
256257
#include "date.h"
@@ -451,6 +452,54 @@ static void reset_cache_server(void)
451452
}
452453
}
453454

455+
/*
456+
* Build the X-Session-Id header value based on gvfs.sessionkey config.
457+
*
458+
* If gvfs.sessionkey is set, it specifies which config key contains
459+
* a prefix to prepend to the SID. The format is: <prefix>:<SID>
460+
*
461+
* If gvfs.sessionkey is not set or the referenced key doesn't exist,
462+
* the header value is just the SID.
463+
*
464+
* Returns a newly allocated string that must be freed by the caller.
465+
*/
466+
static char *build_session_id_header(void)
467+
{
468+
struct strbuf header = STRBUF_INIT;
469+
const char *sid = tr2_sid_get();
470+
char *session_key = NULL;
471+
char *prefix = NULL;
472+
473+
/* Read gvfs.sessionkey to see if it points to a config key */
474+
if (!repo_config_get_string(the_repository, "gvfs.sessionkey", &session_key) &&
475+
session_key) {
476+
/* Try to read the config key that session_key points to */
477+
if (!repo_config_get_string(the_repository, session_key, &prefix) &&
478+
prefix) {
479+
/* We have a prefix, format as: X-Session-Id: <prefix>:<SID> */
480+
strbuf_addf(&header, "X-Session-Id: %s:%s", prefix, sid);
481+
free(prefix);
482+
} else {
483+
/* Config key doesn't exist, use just SID */
484+
strbuf_addf(&header, "X-Session-Id: %s", sid);
485+
}
486+
487+
free(session_key);
488+
} else {
489+
/* No gvfs.sessionkey configured, use just SID */
490+
strbuf_addf(&header, "X-Session-Id: %s", sid);
491+
}
492+
493+
return strbuf_detach(&header, NULL);
494+
}
495+
496+
static void append_session_id_header(struct curl_slist **headers)
497+
{
498+
char *session_id_header = build_session_id_header();
499+
*headers = curl_slist_append(*headers, session_id_header);
500+
free(session_id_header);
501+
}
502+
454503
static const char *gh__server_type_label[GH__SERVER_TYPE__NR] = {
455504
"(main)",
456505
"(cs)"
@@ -3304,6 +3353,7 @@ static void do__http_get__simple_endpoint(struct gh__response_status *status,
33043353
"X-TFS-FedAuthRedirect: Suppress");
33053354
params.headers = curl_slist_append(params.headers,
33063355
"Pragma: no-cache");
3356+
append_session_id_header(&params.headers);
33073357

33083358
if (gh__cmd_opts.show_progress) {
33093359
/*
@@ -3373,6 +3423,7 @@ static void do__http_get__gvfs_object(struct gh__response_status *status,
33733423
"X-TFS-FedAuthRedirect: Suppress");
33743424
params.headers = curl_slist_append(params.headers,
33753425
"Pragma: no-cache");
3426+
append_session_id_header(&params.headers);
33763427

33773428
oidcpy(&params.loose_oid, oid);
33783429

@@ -3434,6 +3485,8 @@ static void do__http_post__gvfs_objects(struct gh__response_status *status,
34343485
"Pragma: no-cache");
34353486
params.headers = curl_slist_append(params.headers,
34363487
"Content-Type: application/json");
3488+
append_session_id_header(&params.headers);
3489+
34373490
/*
34383491
* If our POST contains more than one object, we want the
34393492
* server to send us a packfile. We DO NOT want the non-standard
@@ -3570,6 +3623,7 @@ static void do__http_get__gvfs_prefetch(struct gh__response_status *status,
35703623
"Pragma: no-cache");
35713624
params.headers = curl_slist_append(params.headers,
35723625
"Accept: application/x-gvfs-timestamped-packfiles-indexes");
3626+
append_session_id_header(&params.headers);
35733627

35743628
if (gh__cmd_opts.show_progress)
35753629
strbuf_addf(&params.progress_base_phase3_msg,

t/helper/test-gvfs-protocol.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,19 @@ static enum worker_result req__read(struct req *req, int fd)
15521552
*/
15531553
done:
15541554

1555+
/*
1556+
* Log the X-Session-Id header if present (for testing purposes).
1557+
*/
1558+
{
1559+
struct string_list_item *item;
1560+
for_each_string_list_item(item, &req->header_list) {
1561+
if (starts_with(item->string, "X-Session-Id:")) {
1562+
loginfo("Received header: %s", item->string);
1563+
break;
1564+
}
1565+
}
1566+
}
1567+
15551568
#if 0
15561569
/*
15571570
* This is useful for debugging the request, but very noisy.

t/t5793-gvfs-helper-integration.sh

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,54 @@ test_expect_success 'integration: implicit-get: cache_http_503,with-fallback: di
163163

164164
# T2 should be considered contaminated at this point.
165165

166+
#################################################################
167+
# Test X-Session-Id header
168+
#
169+
# The X-Session-Id header should contain the SID (session ID).
170+
#################################################################
171+
172+
test_expect_success 'integration: X-Session-Id header with and without prefix' '
173+
test_when_finished "per_test_cleanup" &&
174+
start_gvfs_protocol_server &&
175+
176+
# Case 1: No gvfs.sessionkey configured - should send just SID
177+
git -C "$REPO_T1" gvfs-helper \
178+
--cache-server=disable \
179+
--remote=origin \
180+
get \
181+
<"$OID_ONE_BLOB_FILE" >OUT.output1 &&
182+
183+
# Verify X-Session-Id contains SID (with process ID marker "-P")
184+
test_grep "X-Session-Id:.*-P" "$SERVER_LOG" >OUT.case1 &&
185+
# Verify no slash (no prefix)
186+
test_grep ! "X-Session-Id:.*:" OUT.case1 &&
187+
188+
# Case 2: gvfs.sessionkey points to non-existent config - should send just SID
189+
rm -f OUT.output* OUT.case* &&
190+
git -C "$REPO_T1" -c gvfs.sessionkey="test.id" gvfs-helper \
191+
--cache-server=disable \
192+
--remote=origin \
193+
get \
194+
<"$OID_ONE_BLOB_FILE" >OUT.output2 &&
195+
196+
# Verify X-Session-Id still contains just SID (no prefix)
197+
test_grep "X-Session-Id:.*-P" "$SERVER_LOG" >OUT.case2 &&
198+
test_grep ! "X-Session-Id:.*:" OUT.case2 &&
199+
200+
# Case 3: gvfs.sessionkey points to existing config - should send prefix/SID
201+
rm -f OUT.output* OUT.case* &&
202+
git -C "$REPO_T1" \
203+
-c gvfs.sessionkey="test.id" \
204+
-c test.id="my-trace-12345" \
205+
gvfs-helper \
206+
--cache-server=disable \
207+
--remote=origin \
208+
get \
209+
<"$OID_ONE_BLOB_FILE" >OUT.output3 &&
210+
211+
# Verify X-Session-Id contains prefix, slash, and SID
212+
test_grep "X-Session-Id:.*my-trace-12345:" "$SERVER_LOG" >OUT.case3 &&
213+
test_grep "X-Session-Id:.*my-trace-12345:.*-P" OUT.case3
214+
'
215+
166216
test_done

0 commit comments

Comments
 (0)