Skip to content
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.

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

Downgrade the Warning to Information on missing Content-Length header in MultiplexingMiddleware #2146

Merged
merged 4 commits into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 4 additions & 9 deletions src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ public PollyQoSResiliencePipelineProvider(
};

protected virtual HashSet<HttpStatusCode> ServerErrorCodes { get; } = DefaultServerErrorCodes;

protected virtual string GetRouteName(DownstreamRoute route)
=> string.IsNullOrWhiteSpace(route.ServiceName)
? route.UpstreamPathTemplate?.Template ?? route.DownstreamPathTemplate?.Value ?? string.Empty
: route.ServiceName;
protected virtual string GetRouteName(DownstreamRoute route) => route.Name();

/// <summary>
/// Gets Polly V8 resilience pipeline (applies QoS feature) for the route.
Expand All @@ -57,9 +53,8 @@ public ResiliencePipeline<HttpResponseMessage> GetResiliencePipeline(DownstreamR
return ResiliencePipeline<HttpResponseMessage>.Empty; // shortcut -> No QoS
}

var currentRouteName = GetRouteName(route);
return _registry.GetOrAddPipeline<HttpResponseMessage>(
key: new OcelotResiliencePipelineKey(currentRouteName),
key: new OcelotResiliencePipelineKey(GetRouteName(route)),
configure: (builder) => ConfigureStrategies(builder, route));
}

Expand All @@ -78,7 +73,7 @@ protected virtual ResiliencePipelineBuilder<HttpResponseMessage> ConfigureCircui
}

var options = route.QosOptions;
var info = $"Circuit Breaker for Route: {GetRouteName(route)}: ";
var info = $"Circuit Breaker for the route: {GetRouteName(route)}: ";
var strategyOptions = new CircuitBreakerStrategyOptions<HttpResponseMessage>
{
FailureRatio = 0.8,
Expand Down Expand Up @@ -127,7 +122,7 @@ protected virtual ResiliencePipelineBuilder<HttpResponseMessage> ConfigureTimeou
Timeout = TimeSpan.FromMilliseconds(options.TimeoutValue),
OnTimeout = _ =>
{
_logger.LogInformation($"Timeout for Route: {GetRouteName(route)}");
_logger.LogInformation(() => $"Timeout for the route: {GetRouteName(route)}");
return ValueTask.CompletedTask;
},
};
Expand Down
8 changes: 7 additions & 1 deletion src/Ocelot/Configuration/DownstreamRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public DownstreamRoute(
public string ServiceName { get; }
public string ServiceNamespace { get; }
public HttpHandlerOptions HttpHandlerOptions { get; }
public bool UseServiceDiscovery { get; }
public bool EnableEndpointEndpointRateLimiting { get; }
public QoSOptions QosOptions { get; }
public string DownstreamScheme { get; }
Expand Down Expand Up @@ -130,6 +129,13 @@ public DownstreamRoute(
/// </remarks>
public HttpVersionPolicy DownstreamHttpVersionPolicy { get; }
public Dictionary<string, UpstreamHeaderTemplate> UpstreamHeaders { get; }
public bool UseServiceDiscovery { get; }
public MetadataOptions MetadataOptions { get; }

/// <summary>Gets the route name depending on whether the service discovery mode is enabled or disabled.</summary>
/// <returns>A <see cref="string"/> object with the name.</returns>
public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscovery
? UpstreamPathTemplate?.Template ?? DownstreamPathTemplate?.Value ?? "?"
: string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template);
}
}
21 changes: 11 additions & 10 deletions src/Ocelot/Multiplexer/MultiplexingMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private IEnumerable<Task<HttpContext>> ProcessRouteWithComplexAggregation(Aggreg
/// <returns>The cloned Http context.</returns>
private async Task<HttpContext> ProcessRouteAsync(HttpContext sourceContext, DownstreamRoute route, List<PlaceholderNameAndValue> placeholders = null)
{
var newHttpContext = await CreateThreadContextAsync(sourceContext);
var newHttpContext = await CreateThreadContextAsync(sourceContext, route);
CopyItemsToNewContext(newHttpContext, sourceContext, placeholders);
newHttpContext.Items.UpsertDownstreamRoute(route);

Expand All @@ -200,17 +200,18 @@ private static void CopyItemsToNewContext(HttpContext target, HttpContext source
target.Items.SetIInternalConfiguration(source.Items.IInternalConfiguration());
target.Items.UpsertTemplatePlaceholderNameAndValues(placeholders ??
source.Items.TemplatePlaceholderNameAndValues());
}

}
/// <summary>
/// Creates a new HttpContext based on the source.
/// </summary>
/// <param name="source">The base http context.</param>
/// <param name="source">The base http context.</param>
/// <param name="route">Downstream route.</param>
/// <returns>The cloned context.</returns>
protected virtual async Task<HttpContext> CreateThreadContextAsync(HttpContext source)
protected virtual async Task<HttpContext> CreateThreadContextAsync(HttpContext source, DownstreamRoute route)
{
var from = source.Request;
var bodyStream = await CloneRequestBodyAsync(from, source.RequestAborted);
var bodyStream = await CloneRequestBodyAsync(from, route, source.RequestAborted);
var target = new DefaultHttpContext
{
Request =
Expand Down Expand Up @@ -245,7 +246,7 @@ protected virtual async Task<HttpContext> CreateThreadContextAsync(HttpContext s
// Once the downstream request is completed and the downstream response has been read, the downstream response object can dispose of the body's Stream object
target.Response.RegisterForDisposeAsync(bodyStream); // manage Stream lifetime by HttpResponse object
return target;
}
}

protected virtual Task MapAsync(HttpContext httpContext, Route route, List<HttpContext> contexts)
{
Expand All @@ -258,12 +259,12 @@ protected virtual Task MapAsync(HttpContext httpContext, Route route, List<HttpC
return aggregator.Aggregate(route, httpContext, contexts);
}

protected virtual async Task<Stream> CloneRequestBodyAsync(HttpRequest request, CancellationToken aborted)
protected virtual async Task<Stream> CloneRequestBodyAsync(HttpRequest request, DownstreamRoute route, CancellationToken aborted)
{
request.EnableBuffering();
if (request.Body.Position != 0)
{
Logger.LogWarning("Ocelot does not support body copy without stream in initial position 0");
Logger.LogWarning(() => $"Ocelot does not support body copy without stream in initial position 0 for the route {route.Name()}.");
return request.Body;
}

Expand All @@ -276,7 +277,7 @@ protected virtual async Task<Stream> CloneRequestBodyAsync(HttpRequest request,
}
else
{
Logger.LogWarning("Aggregation does not support body copy without Content-Length header!");
Logger.LogInformation(() => $"Aggregation does not support body copy without Content-Length header, skipping body copy for the route {route.Name()}.");
}

return targetBuffer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ public Response<IServiceDiscoveryProvider> Get(ServiceProviderConfiguration serv
{
if (route.UseServiceDiscovery)
{
var routeName = route.UpstreamPathTemplate?.Template ?? route.ServiceName ?? string.Empty;
_logger.LogInformation(() => $"The {nameof(DownstreamRoute.UseServiceDiscovery)} mode of the route '{routeName}' is enabled.");
_logger.LogInformation(() => $"The {nameof(DownstreamRoute.UseServiceDiscovery)} mode of the route '{route.Name()}' is enabled.");
return GetServiceDiscoveryProvider(serviceConfig, route);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,14 @@ public void should_not_multiplex()
[Trait("Bug", "1396")]
public async Task CreateThreadContextAsync_CopyUser_ToTarget()
{
var route = new DownstreamRouteBuilder().Build();

// Arrange
GivenUser("test", "Copy", nameof(CreateThreadContextAsync_CopyUser_ToTarget));

// Act
var method = _middleware.GetType().GetMethod("CreateThreadContextAsync", BindingFlags.NonPublic | BindingFlags.Instance);
var actual = await (Task<HttpContext>)method.Invoke(_middleware, new object[] { _httpContext });
var actual = await (Task<HttpContext>)method.Invoke(_middleware, new object[] { _httpContext, route });

// Assert
AssertUsers(actual);
Expand Down Expand Up @@ -234,6 +236,7 @@ public async Task Should_Call_CloneRequestBodyAsync_Each_Time_Per_Requests(int n
mock.Protected().Verify<Task<Stream>>("CloneRequestBodyAsync",
numberOfRoutes > 1 ? Times.Exactly(numberOfRoutes) : Times.Never(),
ItExpr.IsAny<HttpRequest>(),
ItExpr.IsAny<DownstreamRoute>(),
ItExpr.IsAny<CancellationToken>());
}

Expand Down