Skip to content

Record HTTP status code on blob-hydration failure telemetry #2085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tyrielv merged 1 commit into from
Aug 14, 2026
Merged

Record HTTP status code on blob-hydration failure telemetry #2085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 68 additions & 19 deletions GVFS/GVFS.Common/Git/GVFSGitObjects.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using GVFS.Common.Http;
using GVFS.Common.Http;
using GVFS.Common.Tracing;
using System;
using System.Collections.Concurrent;
Expand All @@ -15,14 +15,14 @@ public class GVFSGitObjects : GitObjects
private static readonly TimeSpan NegativeCacheTTL = TimeSpan.FromSeconds(30);

private ConcurrentDictionary<string, DateTime> objectNegativeCache;
internal ConcurrentDictionary<string, Lazy<DownloadAndSaveObjectResult>> inflightDownloads;
internal ConcurrentDictionary<string, Lazy<DownloadAttemptResult>> inflightDownloads;

public GVFSGitObjects(GVFSContext context, GitObjectsHttpRequestor objectRequestor)
: base(context.Tracer, context.Enlistment, objectRequestor, context.FileSystem)
{
this.Context = context;
this.objectNegativeCache = new ConcurrentDictionary<string, DateTime>(StringComparer.OrdinalIgnoreCase);
this.inflightDownloads = new ConcurrentDictionary<string, Lazy<DownloadAndSaveObjectResult>>(StringComparer.OrdinalIgnoreCase);
this.inflightDownloads = new ConcurrentDictionary<string, Lazy<DownloadAttemptResult>>(StringComparer.OrdinalIgnoreCase);
}

public enum RequestSource
Expand Down Expand Up @@ -58,6 +58,28 @@ public enum BlobHydrationFailureCategory
Unexpected, // Unclassified exception.
}

/// <summary>
/// Carries the outcome of an object download together with the HTTP status of the last
/// download attempt. The public <see cref="DownloadAndSaveObjectResult"/> enum only records
/// success/not-found/error, which collapses genuine auth failures (401/400/302) and
/// transient failures (408/5xx/pool-exhaustion 503) into a single "error" outcome. The
/// status is retained here so the terminal blob-hydration telemetry can tell them apart.
/// </summary>
internal class DownloadAttemptResult
{
public DownloadAttemptResult(DownloadAndSaveObjectResult result, HttpStatusCode? httpStatusCode)
{
this.Result = result;
this.HttpStatusCode = httpStatusCode;
}

public DownloadAndSaveObjectResult Result { get; }

// The HTTP status of the last download attempt, or null when no HTTP response was
// received (for example an exhausted retry that ended in an exception).
public HttpStatusCode? HttpStatusCode { get; }
}

protected GVFSContext Context { get; private set; }

public virtual bool TryCopyBlobContentStream(
Expand All @@ -72,7 +94,7 @@ public virtual bool TryCopyBlobContentStream(
// local copy) that is otherwise collapsed into the bool return value below. The
// final category is also surfaced via the out parameter so the caller can tag its
// own terminal telemetry with the same cause.
DownloadAndSaveObjectResult lastDownloadResult = DownloadAndSaveObjectResult.Error;
DownloadAttemptResult lastDownloadResult = null;
bool downloadSucceededButCopyFailed = false;
BlobHydrationFailureCategory capturedCategory = BlobHydrationFailureCategory.None;

Expand Down Expand Up @@ -109,7 +131,7 @@ public virtual bool TryCopyBlobContentStream(
{
category = BlobHydrationFailureCategory.LocalCopyFailed;
}
else if (lastDownloadResult == DownloadAndSaveObjectResult.ObjectNotOnServer)
else if (lastDownloadResult?.Result == DownloadAndSaveObjectResult.ObjectNotOnServer)
{
category = BlobHydrationFailureCategory.ObjectNotOnServer;
}
Expand All @@ -124,6 +146,22 @@ public virtual bool TryCopyBlobContentStream(
capturedCategory = category;
metadata.Add(nameof(BlobHydrationFailureCategory), category.ToString());

// Surface the HTTP status of the last download attempt so telemetry can tell a
// genuine auth failure (401/400/302) apart from a transient one (408/5xx/503),
// both of which otherwise land in the DownloadFailed bucket. Only attach it when
// the failure is attributable to the download itself (DownloadFailed or
// ObjectNotOnServer). On the exception (LocalIO/NetworkUnavailable) and
// LocalCopyFailed paths lastDownloadResult can hold a status captured on an
// earlier attempt, so the status would be stale and misattribute the failure.
bool statusIsAttributable =
category == BlobHydrationFailureCategory.DownloadFailed ||
category == BlobHydrationFailureCategory.ObjectNotOnServer;
if (statusIsAttributable && lastDownloadResult?.HttpStatusCode != null)
{
metadata.Add("HttpStatusCode", (int)lastDownloadResult.HttpStatusCode.Value);
metadata.Add("HttpStatusName", lastDownloadResult.HttpStatusCode.Value.ToString());
}

string message = "TryCopyBlobContentStream: Failed to provide blob contents";
if (errorArgs.WillRetry)
{
Expand All @@ -149,7 +187,7 @@ public virtual bool TryCopyBlobContentStream(

// Pass in false for retryOnFailure because the retrier in this method manages multiple attempts
lastDownloadResult = this.TryDownloadAndSaveObject(sha, cancellationToken, requestSource, retryOnFailure: false);
if (lastDownloadResult == DownloadAndSaveObjectResult.Success)
if (lastDownloadResult.Result == DownloadAndSaveObjectResult.Success)
{
if (this.Context.Repository.TryCopyBlobContentStream(sha, writeAction))
{
Expand All @@ -169,7 +207,7 @@ public virtual bool TryCopyBlobContentStream(

public DownloadAndSaveObjectResult TryDownloadAndSaveObject(string objectId, RequestSource requestSource)
{
return this.TryDownloadAndSaveObject(objectId, CancellationToken.None, requestSource, retryOnFailure: true);
return this.TryDownloadAndSaveObject(objectId, CancellationToken.None, requestSource, retryOnFailure: true).Result;
}

public bool TryGetBlobSizeLocally(string sha, out long length)
Expand All @@ -182,23 +220,23 @@ public bool TryGetBlobSizeLocally(string sha, out long length)
return this.GitObjectRequestor.QueryForFileSizes(objectIds, cancellationToken);
}

private DownloadAndSaveObjectResult TryDownloadAndSaveObject(
private DownloadAttemptResult TryDownloadAndSaveObject(
string objectId,
CancellationToken cancellationToken,
RequestSource requestSource,
bool retryOnFailure)
{
if (objectId == GVFSConstants.AllZeroSha)
{
return DownloadAndSaveObjectResult.Error;
return new DownloadAttemptResult(DownloadAndSaveObjectResult.Error, httpStatusCode: null);
}

DateTime negativeCacheRequestTime;
if (this.objectNegativeCache.TryGetValue(objectId, out negativeCacheRequestTime))
{
if (negativeCacheRequestTime > DateTime.Now.Subtract(NegativeCacheTTL))
{
return DownloadAndSaveObjectResult.ObjectNotOnServer;
return new DownloadAttemptResult(DownloadAndSaveObjectResult.ObjectNotOnServer, httpStatusCode: null);
}

this.objectNegativeCache.TryRemove(objectId, out negativeCacheRequestTime);
Expand All @@ -210,9 +248,9 @@ private DownloadAndSaveObjectResult TryDownloadAndSaveObject(
// captured by the Lazy factory. Subsequent coalesced callers inherit those
// settings. In practice this is fine because the primary concurrent path
// (NamedPipeMessage from git.exe) always uses CancellationToken.None.
Lazy<DownloadAndSaveObjectResult> newLazy = new Lazy<DownloadAndSaveObjectResult>(
Lazy<DownloadAttemptResult> newLazy = new Lazy<DownloadAttemptResult>(
() => this.DoDownloadAndSaveObject(objectId, cancellationToken, requestSource, retryOnFailure));
Lazy<DownloadAndSaveObjectResult> lazy = this.inflightDownloads.GetOrAdd(objectId, newLazy);
Lazy<DownloadAttemptResult> lazy = this.inflightDownloads.GetOrAdd(objectId, newLazy);

if (!ReferenceEquals(lazy, newLazy))
{
Expand Down Expand Up @@ -240,13 +278,13 @@ private DownloadAndSaveObjectResult TryDownloadAndSaveObject(
/// .NET Framework 4.7.1. When we upgrade to .NET 10 (backlog), this can be
/// replaced with ConcurrentDictionary.TryRemove(KeyValuePair).
/// </summary>
private bool TryRemoveInflightDownload(string objectId, Lazy<DownloadAndSaveObjectResult> lazy)
private bool TryRemoveInflightDownload(string objectId, Lazy<DownloadAttemptResult> lazy)
{
return ((ICollection<KeyValuePair<string, Lazy<DownloadAndSaveObjectResult>>>)this.inflightDownloads)
.Remove(new KeyValuePair<string, Lazy<DownloadAndSaveObjectResult>>(objectId, lazy));
return ((ICollection<KeyValuePair<string, Lazy<DownloadAttemptResult>>>)this.inflightDownloads)
.Remove(new KeyValuePair<string, Lazy<DownloadAttemptResult>>(objectId, lazy));
}

private DownloadAndSaveObjectResult DoDownloadAndSaveObject(
private DownloadAttemptResult DoDownloadAndSaveObject(
string objectId,
CancellationToken cancellationToken,
RequestSource requestSource,
Expand All @@ -273,21 +311,32 @@ private DownloadAndSaveObjectResult DoDownloadAndSaveObject(
return new RetryWrapper<GitObjectsHttpRequestor.GitObjectTaskResult>.CallbackResult(new GitObjectsHttpRequestor.GitObjectTaskResult(true));
});

// Capture the HTTP status of the last download attempt when a response was received.
// On failure the requestor propagates the real status (e.g. 401/404/503); on an
// exhausted retry that ended in an exception output.Result is null and no status is
// known. A default (zero) status means the result carried no HTTP response, so it is
// treated as "no status".
HttpStatusCode? httpStatusCode = null;
if (output.Result != null && output.Result.HttpStatusCodeResult != 0)
{
httpStatusCode = output.Result.HttpStatusCodeResult;
}

if (output.Result != null)
{
if (output.Succeeded && output.Result.Success)
{
return DownloadAndSaveObjectResult.Success;
return new DownloadAttemptResult(DownloadAndSaveObjectResult.Success, httpStatusCode);
}

if (output.Result.HttpStatusCodeResult == HttpStatusCode.NotFound)
{
this.objectNegativeCache.AddOrUpdate(objectId, DateTime.Now, (unused1, unused2) => DateTime.Now);
return DownloadAndSaveObjectResult.ObjectNotOnServer;
return new DownloadAttemptResult(DownloadAndSaveObjectResult.ObjectNotOnServer, httpStatusCode);
}
}

return DownloadAndSaveObjectResult.Error;
return new DownloadAttemptResult(DownloadAndSaveObjectResult.Error, httpStatusCode);
}
}
}
Loading
Loading