diff --git a/README.md b/README.md index 3ef6d6b6..dfbdfd1e 100644 --- a/README.md +++ b/README.md @@ -305,19 +305,25 @@ Below is an example configuration that will transforms claims to query string pa This shows a transform where Ocelot looks at the users LocationId claim and add its as a query string parameter to be forwarded onto the downstream service. -## Logging +## Quality of Service -Ocelot uses the standard logging interfaces ILoggerFactory / ILogger at the moment. -This is encapsulated in IOcelotLogger / IOcelotLoggerFactory with an implementation -for the standard asp.net core logging stuff at the moment. +Ocelot supports one QoS capability at the current time. You can set on a per ReRoute basis if you +want to use a circuit breaker when making requests to a downstream service. This uses the an awesome +.NET library called Polly check them out [here](https://github.com/App-vNext/Polly). -There are a bunch of debugging logs in the ocelot middlewares however I think the -system probably needs more logging in the code it calls into. Other than the debugging -there is a global error handler that should catch any errors thrown and log them as errors. +Add the following section to a ReRoute configuration. -The reason for not just using bog standard framework logging is that I could not -work out how to override the request id that get's logged when setting IncludeScopes -to true for logging settings. Nicely onto the next feature. + "QoSOptions": { + "ExceptionsAllowedBeforeBreaking":3, + "DurationOfBreak":5, + "TimeoutValue":5000 + } + +You must set a number greater than 0 against ExceptionsAllowedBeforeBreaking for this rule to be +implemented. Duration of break is how long the circuit breaker will stay open for after it is tripped. +TimeoutValue means ff a request takes more than 5 seconds it will automatically be timed out. + +If you do not add a QoS section QoS will not be used. ## RequestId / CorrelationId @@ -404,6 +410,20 @@ http request before it is passed to Ocelots request creator. Obviously you can just add middleware as normal before the call to app.UseOcelot() It cannot be added after as Ocelot does not call the next middleware. +## Logging + +Ocelot uses the standard logging interfaces ILoggerFactory / ILogger at the moment. +This is encapsulated in IOcelotLogger / IOcelotLoggerFactory with an implementation +for the standard asp.net core logging stuff at the moment. + +There are a bunch of debugging logs in the ocelot middlewares however I think the +system probably needs more logging in the code it calls into. Other than the debugging +there is a global error handler that should catch any errors thrown and log them as errors. + +The reason for not just using bog standard framework logging is that I could not +work out how to override the request id that get's logged when setting IncludeScopes +to true for logging settings. Nicely onto the next feature. + ## Not supported Ocelot does not support... diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 8584c57e..9deaaeda 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -8,7 +8,11 @@ namespace Ocelot.Configuration { public class QoSOptions { - public QoSOptions(int exceptionsAllowedBeforeBreaking, int durationofBreak, int timeoutValue, TimeoutStrategy timeoutStrategy = TimeoutStrategy.Pessimistic) + public QoSOptions( + int exceptionsAllowedBeforeBreaking, + int durationofBreak, + int timeoutValue, + TimeoutStrategy timeoutStrategy = TimeoutStrategy.Pessimistic) { ExceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking; DurationOfBreak = TimeSpan.FromMilliseconds(durationofBreak); diff --git a/src/Ocelot/Errors/OcelotErrorCode.cs b/src/Ocelot/Errors/OcelotErrorCode.cs index d24988b9..e2fe4ada 100644 --- a/src/Ocelot/Errors/OcelotErrorCode.cs +++ b/src/Ocelot/Errors/OcelotErrorCode.cs @@ -25,6 +25,7 @@ ServicesAreNullError, ServicesAreEmptyError, UnableToFindServiceDiscoveryProviderError, - UnableToFindLoadBalancerError + UnableToFindLoadBalancerError, + RequestTimedOutError } } diff --git a/src/Ocelot/Requester/CircuitBreakingDelegatingHandler.cs b/src/Ocelot/Requester/CircuitBreakingDelegatingHandler.cs index 4adc0eeb..b4091296 100644 --- a/src/Ocelot/Requester/CircuitBreakingDelegatingHandler.cs +++ b/src/Ocelot/Requester/CircuitBreakingDelegatingHandler.cs @@ -18,11 +18,17 @@ namespace Ocelot.Requester private readonly Policy _circuitBreakerPolicy; private readonly TimeoutPolicy _timeoutPolicy; - public CircuitBreakingDelegatingHandler(int exceptionsAllowedBeforeBreaking, TimeSpan durationOfBreak,TimeSpan timeoutValue - ,TimeoutStrategy timeoutStrategy, IOcelotLogger logger, HttpMessageHandler innerHandler) + public CircuitBreakingDelegatingHandler( + int exceptionsAllowedBeforeBreaking, + TimeSpan durationOfBreak, + TimeSpan timeoutValue, + TimeoutStrategy timeoutStrategy, + IOcelotLogger logger, + HttpMessageHandler innerHandler) : base(innerHandler) { this._exceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking; + this._durationOfBreak = durationOfBreak; _circuitBreakerPolicy = Policy @@ -39,30 +45,27 @@ namespace Ocelot.Requester onReset: () => _logger.LogDebug(".Breaker logging: Call ok! Closed the circuit again."), onHalfOpen: () => _logger.LogDebug(".Breaker logging: Half-open; next call is a trial.") ); + _timeoutPolicy = Policy.TimeoutAsync(timeoutValue, timeoutStrategy); + _logger = logger; } - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - Task responseTask = null; - try { - responseTask = Policy.WrapAsync(_circuitBreakerPolicy, _timeoutPolicy).ExecuteAsync(() => - { - return base.SendAsync(request,cancellationToken); - }); - return responseTask; + return await Policy.WrapAsync(_circuitBreakerPolicy, _timeoutPolicy).ExecuteAsync(() => base.SendAsync(request,cancellationToken)); } catch (BrokenCircuitException ex) { _logger.LogError($"Reached to allowed number of exceptions. Circuit is open. AllowedExceptionCount: {_exceptionsAllowedBeforeBreaking}, DurationOfBreak: {_durationOfBreak}",ex); throw; } - catch (HttpRequestException) + catch (HttpRequestException ex) { - return responseTask; + _logger.LogError($"Error in CircuitBreakingDelegatingHandler.SendAync", ex); + throw; } } diff --git a/src/Ocelot/Requester/HttpClientHttpRequester.cs b/src/Ocelot/Requester/HttpClientHttpRequester.cs index 77ab90cc..cc5332f3 100644 --- a/src/Ocelot/Requester/HttpClientHttpRequester.cs +++ b/src/Ocelot/Requester/HttpClientHttpRequester.cs @@ -27,6 +27,7 @@ namespace Ocelot.Requester { builder.WithCircuitBreaker(request.Qos, _logger, handler); } + using (var httpClient = builder.Build(handler)) { try @@ -34,13 +35,14 @@ namespace Ocelot.Requester var response = await httpClient.SendAsync(request.HttpRequestMessage); return new OkResponse(response); } - catch (Exception exception) + catch (Polly.Timeout.TimeoutRejectedException exception) { return - new ErrorResponse(new List - { - new UnableToCompleteRequestError(exception) - }); + new ErrorResponse(new RequestTimedOutError(exception)); + } + catch (Exception exception) + { + return new ErrorResponse(new UnableToCompleteRequestError(exception)); } } } diff --git a/src/Ocelot/Requester/RequestTimedOutError.cs b/src/Ocelot/Requester/RequestTimedOutError.cs new file mode 100644 index 00000000..38afce1a --- /dev/null +++ b/src/Ocelot/Requester/RequestTimedOutError.cs @@ -0,0 +1,13 @@ +using System; +using Ocelot.Errors; + +namespace Ocelot.Requester +{ + public class RequestTimedOutError : Error + { + public RequestTimedOutError(Exception exception) + : base($"Timeout making http request, exception: {exception.Message}", OcelotErrorCode.RequestTimedOutError) + { + } + } +} diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 6ecc20eb..b9fcb299 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -22,6 +22,11 @@ namespace Ocelot.Responder return new OkResponse(403); } + if (errors.Any(e => e.Code == OcelotErrorCode.RequestTimedOutError)) + { + return new OkResponse(408); + } + return new OkResponse(404); } } diff --git a/src/Ocelot/Responses/ErrorResponseGeneric.cs b/src/Ocelot/Responses/ErrorResponseGeneric.cs index 77230503..044d6f61 100644 --- a/src/Ocelot/Responses/ErrorResponseGeneric.cs +++ b/src/Ocelot/Responses/ErrorResponseGeneric.cs @@ -5,7 +5,13 @@ namespace Ocelot.Responses { public class ErrorResponse : Response { - public ErrorResponse(List errors) : base(errors) + public ErrorResponse(Error error) + : base(new List {error}) + { + + } + public ErrorResponse(List errors) + : base(errors) { } } diff --git a/test/Ocelot.AcceptanceTests/QoSTests.cs b/test/Ocelot.AcceptanceTests/QoSTests.cs new file mode 100644 index 00000000..50c1fb66 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/QoSTests.cs @@ -0,0 +1,136 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Net; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration.File; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.AcceptanceTests +{ + public class QoSTests : IDisposable + { + private IWebHost _builder; + private readonly Steps _steps; + private int _requestCount; + + public QoSTests() + { + _steps = new Steps(); + } + + + [Fact] + public void should_open_circuit_breaker_then_close() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "http", + DownstreamHost = "localhost", + DownstreamPort = 51879, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = "Get", + QoSOptions = new FileQoSOptions + { + ExceptionsAllowedBeforeBreaking = 1, + TimeoutValue = 500, + DurationOfBreak = 1000 + } + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51879", "Hello from Laura")) + .Given(x => _steps.GivenThereIsAConfiguration(configuration)) + .Given(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .Given(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Given(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.RequestTimeout)) + .Given(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Given(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.RequestTimeout)) + .Given(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Given(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.RequestTimeout)) + .Given(x => x.GivenIWaitMilliSeconds(2000)) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .BDDfy(); + } + + private void GivenIWaitMilliSeconds(int ms) + { + Thread.Sleep(ms); + } + + private void GivenThereIsAServiceRunningOn(string url, string responseBody) + { + _builder = new WebHostBuilder() + .UseUrls(url) + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseIISIntegration() + .UseUrls(url) + .Configure(app => + { + app.Run(async context => + { + //circuit starts closed + if (_requestCount == 0) + { + _requestCount++; + context.Response.StatusCode = 200; + await context.Response.WriteAsync(responseBody); + return; + } + + //request one times out and polly throws exception + if (_requestCount == 1) + { + _requestCount++; + await Task.Delay(1000); + context.Response.StatusCode = 200; + return; + } + + //request two times out and polly throws exception circuit opens + if (_requestCount == 2) + { + _requestCount++; + await Task.Delay(1000); + context.Response.StatusCode = 200; + return; + } + + //after break closes we return 200 OK + if (_requestCount == 3) + { + context.Response.StatusCode = 200; + await context.Response.WriteAsync(responseBody); + return; + } + }); + }) + .Build(); + + _builder.Start(); + } + + public void Dispose() + { + _builder?.Dispose(); + _steps.Dispose(); + } + } +} diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index cb8198dc..d2b7e386 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -1,6 +1,8 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using Ocelot.Errors; using Ocelot.Middleware; +using Ocelot.Requester; using Ocelot.Responder; using Ocelot.Responses; using Shouldly; @@ -20,6 +22,18 @@ namespace Ocelot.UnitTests.Responder _codeMapper = new ErrorsToHttpStatusCodeMapper(); } + [Fact] + public void should_return_timeout() + { + this.Given(x => x.GivenThereAreErrors(new List + { + new RequestTimedOutError(new Exception()) + })) + .When(x => x.WhenIGetErrorStatusCode()) + .Then(x => x.ThenTheResponseIsStatusCodeIs(408)) + .BDDfy(); + } + [Fact] public void should_create_unauthenticated_response_code() {