Skip to content

Commit

Permalink
Merge pull request #842 from dlcs/feature/orchestrator_timeout
Browse files Browse the repository at this point in the history
Allow ActivityTimeout to be set per destination
  • Loading branch information
donaldgray committed May 8, 2024
2 parents 2ff020f + 8f41d41 commit 814da86
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,7 @@ public async Task Get_ImageRequiresAuth_RedirectsToImageServer_AndSetsCookie_IfC
response.StatusCode.Should().Be(HttpStatusCode.OK);
response.Headers.Should().ContainKey("Set-Cookie");
response.Headers.Should().ContainKey("x-asset-id").WhoseValue.Should().ContainSingle(id.ToString());
proxyResponse.ActivityTimeout.Should().BeCloseTo(TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(2));
}

[Theory]
Expand Down Expand Up @@ -1464,6 +1465,7 @@ public async Task Get_RedirectsSpecialServer_AsFallThrough_ForFullRequests(strin
response.Headers.CacheControl.MaxAge.Should().Be(TimeSpan.FromDays(28));
response.Headers.Should().ContainKey("x-test-key").WhoseValue.Should().BeEquivalentTo("foo bar");
response.Headers.Should().ContainKey("x-asset-id").WhoseValue.Should().ContainSingle(id.ToString());
proxyResponse.ActivityTimeout.Should().BeCloseTo(TimeSpan.FromSeconds(60), TimeSpan.FromSeconds(2));
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class TestProxyForwarder : IHttpForwarder
await transformer.TransformResponseAsync(context, proxyResponse);

// Write request parameters into response
await context.Response.WriteAsJsonAsync(new ProxyResponse(requestMessage));
await context.Response.WriteAsJsonAsync(new ProxyResponse(requestMessage, requestConfig.ActivityTimeout));
return ForwarderError.None;
}
}
Expand All @@ -72,13 +72,15 @@ public class ProxyResponse
{
public Uri Uri { get; set; }
public HttpMethod Method { get; set;}
public TimeSpan? ActivityTimeout { get; set; }

public ProxyResponse()
{
}

public ProxyResponse(HttpRequestMessage request)
public ProxyResponse(HttpRequestMessage request, TimeSpan? activityTimeout = null)
{
ActivityTimeout = activityTimeout;
Uri = request.RequestUri;
Method = request.Method;
}
Expand Down
3 changes: 3 additions & 0 deletions src/protagonist/Orchestrator.Tests/appsettings.Testing.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
"cantaloupe/one": {
"Address": "http://image-server"
}
},
"HttpRequest": {
"ActivityTimeout": "00:00:10"
}
},
"specialserver": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public static void MapFileHandling(this IEndpointRouteBuilder endpoints)

if (error != ForwarderError.None)
{
error.HandleProxyError(httpContext, logger);
error.HandleProxyError(httpContext, RequestOptions, logger);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ namespace Orchestrator.Features.Images;
public static class ImageRouteHandlers
{
private static readonly HttpMessageInvoker HttpClient;
private static readonly ForwarderRequestConfig RequestOptions;
private static readonly ForwarderRequestConfig DefaultRequestOptions;

static ImageRouteHandlers()
{
// TODO - should this be shared by AV + Image handling?
HttpClient = new HttpMessageInvoker(new SocketsHttpHandler
{
UseProxy = false,
Expand All @@ -34,8 +33,7 @@ static ImageRouteHandlers()
ActivityHeadersPropagator = new ReverseProxyPropagator(DistributedContextPropagator.Current)
});

// TODO - make this configurable, potentially by target
RequestOptions = new ForwarderRequestConfig { ActivityTimeout = TimeSpan.FromSeconds(60) };
DefaultRequestOptions = new ForwarderRequestConfig { ActivityTimeout = TimeSpan.FromSeconds(60) };
}

/// <summary>
Expand Down Expand Up @@ -113,7 +111,7 @@ private static void HandleStatusCodeResult(HttpContext httpContext, StatusCodeRe
return;
}

var destination = destinationSelector.GetClusterTarget(httpContext, cluster!);
var destination = destinationSelector.GetClusterTarget(httpContext, cluster);

if (destination == null)
{
Expand All @@ -125,11 +123,15 @@ private static void HandleStatusCodeResult(HttpContext httpContext, StatusCodeRe
var root = destination.Model.Config.Address;

var transformer = new PathRewriteTransformer(proxyAction);
var error = await forwarder.SendAsync(httpContext, root, HttpClient, RequestOptions, transformer);
var requestOptions = GetRequestOptions(cluster);
var error = await forwarder.SendAsync(httpContext, root, HttpClient, requestOptions, transformer);

if (error != ForwarderError.None)
{
error.HandleProxyError(httpContext, logger);
error.HandleProxyError(httpContext, requestOptions, logger);
}
}

private static ForwarderRequestConfig GetRequestOptions(ClusterState clusterState) =>
clusterState.Model.Config.HttpRequest ?? DefaultRequestOptions;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public static class TimeBasedRouteHandlers

static TimeBasedRouteHandlers()
{
// TODO - should this be shared by AV + Image handling?
HttpClient = new HttpMessageInvoker(new SocketsHttpHandler
{
UseProxy = false,
Expand Down Expand Up @@ -87,7 +86,7 @@ public static void MapTimeBasedHandling(this IEndpointRouteBuilder endpoints)
// Check if the proxy operation was successful
if (error != ForwarderError.None)
{
error.HandleProxyError(httpContext, logger);
error.HandleProxyError(httpContext, RequestOptions, logger);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Diagnostics.CodeAnalysis;
using DLCS.Core.Collections;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -34,14 +34,14 @@ public class DownstreamDestinationSelector
this.proxyStateLookup = proxyStateLookup;
this.orchestratorSettings = orchestratorSettings;
}

/// <summary>
/// Attempt to get <see cref="ClusterState"/> object for specified <see cref="ProxyDestination"/>
/// </summary>
/// <param name="destination">Destination to get ClusterState for</param>
/// <param name="clusterState">ClusterState, if found</param>
/// <returns>true if ClusterState found, else false</returns>
public bool TryGetCluster(ProxyDestination destination, out ClusterState? clusterState)
public bool TryGetCluster(ProxyDestination destination, [NotNullWhen(true)] out ClusterState? clusterState)
{
clusterState = null;
if (destination is ProxyDestination.S3 or ProxyDestination.Unknown)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ public static class ReverseProxyHandling
/// <summary>
/// Log any errors that arise during reverse-proxy forwarding operations
/// </summary>
public static void HandleProxyError(this ForwarderError error, HttpContext httpContext, ILogger logger)
public static void HandleProxyError(this ForwarderError error, HttpContext httpContext,
ForwarderRequestConfig requestOptions, ILogger logger)
{
if (error is ForwarderError.RequestCanceled or ForwarderError.RequestBodyCanceled
or ForwarderError.ResponseBodyCanceled or ForwarderError.UpgradeRequestCanceled
Expand All @@ -28,6 +29,18 @@ public static void HandleProxyError(this ForwarderError error, HttpContext httpC
return;
}

if (error is ForwarderError.RequestTimedOut)
{
logger.LogError("Proxy error {Error} for {Path}, ActivityTimeout: {ActivityTimeout}", error,
httpContext.Request.Path, GetActivityTimeoutForLog(requestOptions));
return;
}

logger.LogError(errorFeature.Exception, "Proxy error {Error} for {Path}", error, httpContext.Request.Path);
}

private static string GetActivityTimeoutForLog(ForwarderRequestConfig requestOptions) =>
requestOptions.ActivityTimeout.HasValue
? $"{requestOptions.ActivityTimeout.Value.TotalMilliseconds}ms"
: "default";
}
7 changes: 7 additions & 0 deletions src/protagonist/Orchestrator/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@
}
},
"ReverseProxy": {
"Clusters": {
"cantaloupe": {
"HttpRequest": {
"ActivityTimeout": "00:00:30"
}
}
},
"Routes": {
"thumbs": {
"ClusterId": "thumbs",
Expand Down

0 comments on commit 814da86

Please sign in to comment.