From ff3e7c6665c3a15365221ad296b36e6cdcdf9a9f Mon Sep 17 00:00:00 2001 From: Felix Boers Date: Thu, 5 Apr 2018 20:07:23 +0200 Subject: [PATCH 1/3] Log downstream templates in DownstreamRouteFinderMiddleware (#302) Concatenate all downstream templates separated by , (colon) in order to write them to the log. --- .../Middleware/DownstreamRouteFinderMiddleware.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index c9da50a4..0639135d 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -1,4 +1,5 @@ using System.Threading.Tasks; +using System.Linq; using Ocelot.Configuration; using Ocelot.Configuration.Provider; using Ocelot.DownstreamRouteFinder.Finder; @@ -57,10 +58,10 @@ namespace Ocelot.DownstreamRouteFinder.Middleware SetPipelineError(context, downstreamRoute.Errors); return; - } - - // todo - put this back in - //// _logger.LogDebug("downstream template is {downstreamRoute.Data.ReRoute.DownstreamPath}", downstreamRoute.Data.ReRoute.DownstreamReRoute.DownstreamPathTemplate); + } + + var downstreamPathTemplates = string.Join(", ", downstreamRoute.Data.ReRoute.DownstreamReRoute.Select(r => r.DownstreamPathTemplate.Value)); + _logger.LogDebug($"downstream templates are {downstreamPathTemplates}"); context.TemplatePlaceholderNameAndValues = downstreamRoute.Data.TemplatePlaceholderNameAndValues; From 0c380239a973f5f4cb7c6e9d15013425fe7d5cbe Mon Sep 17 00:00:00 2001 From: Felix Boers Date: Thu, 5 Apr 2018 20:07:45 +0200 Subject: [PATCH 2/3] Override ToString() in DownstreamRequest (#301) When executing the downstreamUrlCreatorMittleware the downstream request url shoud be written to the log. But instead of the url the type name gets written because the ToString() method wasn't overriden. Now ToString() internally calls the ToUri() method in order to provide the url instead of the type name. --- src/Ocelot/Request/Middleware/DownstreamRequest.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Ocelot/Request/Middleware/DownstreamRequest.cs b/src/Ocelot/Request/Middleware/DownstreamRequest.cs index 449b33cc..75070bfd 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequest.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequest.cs @@ -65,5 +65,10 @@ namespace Ocelot.Request.Middleware return uriBuilder.Uri.AbsoluteUri; } + + public override string ToString() + { + return ToUri(); + } } } From efbb950ea2842b448b72c055fe358dc10ecf6710 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Sat, 7 Apr 2018 12:03:24 +0100 Subject: [PATCH 3/3] Feature/another look at logging (#303) * #212 - hacked websockets proxy together * faffing around * #212 hacking away :( * #212 websockets proxy middleware working * #212 map when for webockets working * #212 some test refactor * #212 temp commit * #212 websockets proxy working, tests passing...need to do some tidying and write docs * #212 more code coverage * #212 docs for websockets * #212 updated readme * #212 tidying up after websockets refactoring * #212 tidying up after websockets refactoring * #212 tidying up after websockets refactoring * changing logging levels and logging like ms reccomends with structured data rather than strings * more faffing * more fafin * #287 ocelot logger now only takes strings as it did take params then just turned them to strings, misleading, unit tests for logger and diagnosticlogger * #287 errors now logged as they happen * #287 more detail for logs requested in issue * #287 tidy up * #287 renamed * #287 always log context id * #287 fixed me being an idiot * #287 removed crap websockets unit test that isnt a unit test * #287 removed crap websockets unit test that isnt a unit test --- .../Middleware/AuthenticationMiddleware.cs | 18 +-- src/Ocelot/Authorisation/ClaimsAuthoriser.cs | 13 +- .../Middleware/AuthorisationMiddleware.cs | 33 ++-- src/Ocelot/Authorisation/ScopesAuthoriser.cs | 8 +- .../Cache/Middleware/OutputCacheMiddleware.cs | 15 +- .../Middleware/ClaimsBuilderMiddleware.cs | 7 +- .../Creator/ClaimsToThingCreator.cs | 3 +- .../Creator/HeaderFindAndReplaceCreator.cs | 4 +- .../Configuration/IOcelotConfiguration.cs | 2 +- .../Parser/ClaimToThingConfigurationParser.cs | 18 +-- .../ConsulFileConfigurationPoller.cs | 6 +- .../ConfigurationValidationResult.cs | 4 +- .../Finder/DownstreamRouteFinder.cs | 5 +- .../UnableToFindDownstreamRouteError.cs | 3 +- .../DownstreamRouteFinderMiddleware.cs | 11 +- .../DownstreamUrlCreator/IUrlBuilder.cs | 12 -- .../DownstreamUrlCreatorMiddleware.cs | 7 +- src/Ocelot/DownstreamUrlCreator/UrlBuilder.cs | 47 ------ src/Ocelot/Errors/Error.cs | 2 +- .../Middleware/ExceptionHandlerMiddleware.cs | 46 +++--- src/Ocelot/Headers/AddHeadersToResponse.cs | 2 +- .../HttpHeadersTransformationMiddleware.cs | 3 +- .../HttpRequestHeadersBuilderMiddleware.cs | 11 +- .../Claims/Parser/ClaimsParser.cs | 10 +- .../Extensions/StringValuesExtensions.cs | 3 +- .../RequestData/HttpDataRepository.cs | 20 +-- .../LoadBalancers/LeastConnection.cs | 4 +- .../Middleware/LoadBalancingMiddleware.cs | 9 +- src/Ocelot/Logging/AspDotNetLogger.cs | 36 ++--- src/Ocelot/Logging/AspDotNetLoggerFactory.cs | 14 +- src/Ocelot/Logging/IOcelotLogger.cs | 13 +- .../Logging/OcelotDiagnosticListener.cs | 22 +-- src/Ocelot/Middleware/DownstreamContext.cs | 11 +- src/Ocelot/Middleware/OcelotMiddleware.cs | 19 ++- .../OcelotPipelineBuilderExtensions.cs | 10 +- .../QueryStringBuilderMiddleware.cs | 7 +- src/Ocelot/Raft/RaftController.cs | 13 ++ .../Middleware/ClientRateLimitMiddleware.cs | 12 +- .../DownstreamRequestInitialiserMiddleware.cs | 3 +- .../Middleware/ReRouteRequestIdMiddleware.cs | 23 +-- .../Middleware/HttpRequesterMiddleware.cs | 7 +- .../Requester/OcelotHttpTracingHandler.cs | 10 +- .../Middleware/ResponderMiddleware.cs | 16 +- src/Ocelot/Responses/ErrorResponse.cs | 6 +- src/Ocelot/Responses/Response.cs | 12 +- .../ConsulServiceDiscoveryProvider.cs | 2 +- .../Middleware/WebSocketsProxyMiddleware.cs | 3 +- test/Ocelot.ManualTest/Program.cs | 4 +- test/Ocelot.ManualTest/appsettings.json | 2 +- .../ClaimsToThingCreatorTests.cs | 2 +- .../HeaderFindAndReplaceCreatorTests.cs | 2 +- .../Errors/ExceptionHandlerMiddlewareTests.cs | 48 +++--- .../Headers/AddHeadersToResponseTests.cs | 4 +- .../Logging/AspDotNetLoggerTests.cs | 81 ++++++++++ .../Logging/OcelotDiagnosticListenerTests.cs | 145 ++++++++++++++++++ .../Middleware/OcelotMiddlewareTests.cs | 75 +++++++++ test/Ocelot.UnitTests/Ocelot.UnitTests.csproj | 4 + .../ReRouteRequestIdMiddlewareTests.cs | 45 ++++-- .../Responder/ResponderMiddlewareTests.cs | 2 +- .../ConsulServiceDiscoveryProviderTests.cs | 8 +- 60 files changed, 600 insertions(+), 387 deletions(-) delete mode 100644 src/Ocelot/DownstreamUrlCreator/IUrlBuilder.cs delete mode 100644 src/Ocelot/DownstreamUrlCreator/UrlBuilder.cs create mode 100644 test/Ocelot.UnitTests/Logging/AspDotNetLoggerTests.cs create mode 100644 test/Ocelot.UnitTests/Logging/OcelotDiagnosticListenerTests.cs create mode 100644 test/Ocelot.UnitTests/Middleware/OcelotMiddlewareTests.cs diff --git a/src/Ocelot/Authentication/Middleware/AuthenticationMiddleware.cs b/src/Ocelot/Authentication/Middleware/AuthenticationMiddleware.cs index bd4683d0..2c70e394 100644 --- a/src/Ocelot/Authentication/Middleware/AuthenticationMiddleware.cs +++ b/src/Ocelot/Authentication/Middleware/AuthenticationMiddleware.cs @@ -12,20 +12,19 @@ namespace Ocelot.Authentication.Middleware public class AuthenticationMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; public AuthenticationMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory) + : base(loggerFactory.CreateLogger()) { _next = next; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) { if (IsAuthenticatedRoute(context.DownstreamReRoute)) { - _logger.LogDebug($"{context.HttpContext.Request.Path} is an authenticated route. {MiddlewareName} checking if client is authenticated"); + Logger.LogInformation($"{context.HttpContext.Request.Path} is an authenticated route. {MiddlewareName} checking if client is authenticated"); var result = await context.HttpContext.AuthenticateAsync(context.DownstreamReRoute.AuthenticationOptions.AuthenticationProviderKey); @@ -33,25 +32,22 @@ namespace Ocelot.Authentication.Middleware if (context.HttpContext.User.Identity.IsAuthenticated) { - _logger.LogDebug($"Client has been authenticated for {context.HttpContext.Request.Path}"); + Logger.LogInformation($"Client has been authenticated for {context.HttpContext.Request.Path}"); await _next.Invoke(context); } else { - var error = new List - { - new UnauthenticatedError( - $"Request for authenticated route {context.HttpContext.Request.Path} by {context.HttpContext.User.Identity.Name} was unauthenticated") - }; + var error = new UnauthenticatedError( + $"Request for authenticated route {context.HttpContext.Request.Path} by {context.HttpContext.User.Identity.Name} was unauthenticated"); - _logger.LogError($"Client has NOT been authenticated for {context.HttpContext.Request.Path} and pipeline error set. {error.ToErrorString()}"); + Logger.LogWarning($"Client has NOT been authenticated for {context.HttpContext.Request.Path} and pipeline error set. {error}"); SetPipelineError(context, error); } } else { - _logger.LogTrace($"No authentication needed for {context.HttpContext.Request.Path}"); + Logger.LogInformation($"No authentication needed for {context.HttpContext.Request.Path}"); await _next.Invoke(context); } diff --git a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs index 3ca7809e..d67c4d5f 100644 --- a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs +++ b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs @@ -1,6 +1,5 @@ using System.Collections.Generic; using System.Security.Claims; -using Ocelot.Errors; using Ocelot.Responses; namespace Ocelot.Authorisation @@ -32,19 +31,13 @@ namespace Ocelot.Authorisation var authorised = values.Data.Contains(required.Value); if (!authorised) { - return new ErrorResponse(new List - { - new ClaimValueNotAuthorisedError( - $"claim value: {values.Data} is not the same as required value: {required.Value} for type: {required.Key}") - }); + return new ErrorResponse(new ClaimValueNotAuthorisedError( + $"claim value: {values.Data} is not the same as required value: {required.Value} for type: {required.Key}")); } } else { - return new ErrorResponse(new List - { - new UserDoesNotHaveClaimError($"user does not have claim {required.Key}") - }); + return new ErrorResponse(new UserDoesNotHaveClaimError($"user does not have claim {required.Key}")); } } diff --git a/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs b/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs index 4ace7dbc..82fc3af7 100644 --- a/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs +++ b/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs @@ -13,30 +13,29 @@ private readonly OcelotRequestDelegate _next; private readonly IClaimsAuthoriser _claimsAuthoriser; private readonly IScopesAuthoriser _scopesAuthoriser; - private readonly IOcelotLogger _logger; public AuthorisationMiddleware(OcelotRequestDelegate next, IClaimsAuthoriser claimsAuthoriser, IScopesAuthoriser scopesAuthoriser, IOcelotLoggerFactory loggerFactory) + :base(loggerFactory.CreateLogger()) { _next = next; _claimsAuthoriser = claimsAuthoriser; _scopesAuthoriser = scopesAuthoriser; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) { if (IsAuthenticatedRoute(context.DownstreamReRoute)) { - _logger.LogDebug("route is authenticated scopes must be checked"); + Logger.LogInformation("route is authenticated scopes must be checked"); var authorised = _scopesAuthoriser.Authorise(context.HttpContext.User, context.DownstreamReRoute.AuthenticationOptions.AllowedScopes); if (authorised.IsError) { - _logger.LogDebug("error authorising user scopes"); + Logger.LogWarning("error authorising user scopes"); SetPipelineError(context, authorised.Errors); return; @@ -44,29 +43,26 @@ if (IsAuthorised(authorised)) { - _logger.LogDebug("user scopes is authorised calling next authorisation checks"); + Logger.LogInformation("user scopes is authorised calling next authorisation checks"); } else { - _logger.LogDebug("user scopes is not authorised setting pipeline error"); + Logger.LogWarning("user scopes is not authorised setting pipeline error"); - SetPipelineError(context, new List - { - new UnauthorisedError( - $"{context.HttpContext.User.Identity.Name} unable to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}") - }); + SetPipelineError(context, new UnauthorisedError( + $"{context.HttpContext.User.Identity.Name} unable to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}")); } } if (IsAuthorisedRoute(context.DownstreamReRoute)) { - _logger.LogDebug("route is authorised"); + Logger.LogInformation("route is authorised"); var authorised = _claimsAuthoriser.Authorise(context.HttpContext.User, context.DownstreamReRoute.RouteClaimsRequirement); if (authorised.IsError) { - _logger.LogDebug($"Error whilst authorising {context.HttpContext.User.Identity.Name} for {context.HttpContext.User.Identity.Name}. Setting pipeline error"); + Logger.LogWarning($"Error whilst authorising {context.HttpContext.User.Identity.Name}. Setting pipeline error"); SetPipelineError(context, authorised.Errors); return; @@ -74,22 +70,19 @@ if (IsAuthorised(authorised)) { - _logger.LogDebug($"{context.HttpContext.User.Identity.Name} has succesfully been authorised for {context.DownstreamReRoute.UpstreamPathTemplate.Value}. Calling next middleware"); + Logger.LogInformation($"{context.HttpContext.User.Identity.Name} has succesfully been authorised for {context.DownstreamReRoute.UpstreamPathTemplate.Value}."); await _next.Invoke(context); } else { - _logger.LogDebug($"{context.HttpContext.User.Identity.Name} is not authorised to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}. Setting pipeline error"); + Logger.LogWarning($"{context.HttpContext.User.Identity.Name} is not authorised to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}. Setting pipeline error"); - SetPipelineError(context, new List - { - new UnauthorisedError($"{context.HttpContext.User.Identity.Name} is not authorised to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}") - }); + SetPipelineError(context, new UnauthorisedError($"{context.HttpContext.User.Identity.Name} is not authorised to access {context.DownstreamReRoute.UpstreamPathTemplate.Value}")); } } else { - _logger.LogDebug($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} route does not require user to be authorised"); + Logger.LogInformation($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} route does not require user to be authorised"); await _next.Invoke(context); } } diff --git a/src/Ocelot/Authorisation/ScopesAuthoriser.cs b/src/Ocelot/Authorisation/ScopesAuthoriser.cs index 1654e2a1..4b999c10 100644 --- a/src/Ocelot/Authorisation/ScopesAuthoriser.cs +++ b/src/Ocelot/Authorisation/ScopesAuthoriser.cs @@ -1,5 +1,4 @@ using IdentityModel; -using Ocelot.Errors; using Ocelot.Responses; using System.Collections.Generic; using System.Security.Claims; @@ -38,11 +37,8 @@ namespace Ocelot.Authorisation if (matchesScopes.Count == 0) { - return new ErrorResponse(new List - { - new ScopeNotAuthorisedError( - $"no one user scope: '{string.Join(",", userScopes)}' match with some allowed scope: '{string.Join(",", routeAllowedScopes)}'") - }); + return new ErrorResponse( + new ScopeNotAuthorisedError($"no one user scope: '{string.Join(",", userScopes)}' match with some allowed scope: '{string.Join(",", routeAllowedScopes)}'")); } return new OkResponse(true); diff --git a/src/Ocelot/Cache/Middleware/OutputCacheMiddleware.cs b/src/Ocelot/Cache/Middleware/OutputCacheMiddleware.cs index 88bc4e9e..93548f3b 100644 --- a/src/Ocelot/Cache/Middleware/OutputCacheMiddleware.cs +++ b/src/Ocelot/Cache/Middleware/OutputCacheMiddleware.cs @@ -14,7 +14,6 @@ namespace Ocelot.Cache.Middleware public class OutputCacheMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly IOcelotCache _outputCache; private readonly IRegionCreator _regionCreator; @@ -22,10 +21,10 @@ namespace Ocelot.Cache.Middleware IOcelotLoggerFactory loggerFactory, IOcelotCache outputCache, IRegionCreator regionCreator) + :base(loggerFactory.CreateLogger()) { _next = next; _outputCache = outputCache; - _logger = loggerFactory.CreateLogger(); _regionCreator = regionCreator; } @@ -39,29 +38,29 @@ namespace Ocelot.Cache.Middleware var downstreamUrlKey = $"{context.DownstreamRequest.Method}-{context.DownstreamRequest.OriginalString}"; - _logger.LogDebug("started checking cache for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"Started checking cache for {downstreamUrlKey}"); var cached = _outputCache.Get(downstreamUrlKey, context.DownstreamReRoute.CacheOptions.Region); if (cached != null) { - _logger.LogDebug("cache entry exists for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"cache entry exists for {downstreamUrlKey}"); var response = CreateHttpResponseMessage(cached); SetHttpResponseMessageThisRequest(context, response); - _logger.LogDebug("finished returned cached response for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"finished returned cached response for {downstreamUrlKey}"); return; } - _logger.LogDebug("no resonse cached for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"no resonse cached for {downstreamUrlKey}"); await _next.Invoke(context); if (context.IsError) { - _logger.LogDebug("there was a pipeline error for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"there was a pipeline error for {downstreamUrlKey}"); return; } @@ -70,7 +69,7 @@ namespace Ocelot.Cache.Middleware _outputCache.Add(downstreamUrlKey, cached, TimeSpan.FromSeconds(context.DownstreamReRoute.CacheOptions.TtlSeconds), context.DownstreamReRoute.CacheOptions.Region); - _logger.LogDebug("finished response added to cache for {downstreamUrlKey}", downstreamUrlKey); + Logger.LogDebug($"finished response added to cache for {downstreamUrlKey}"); } private void SetHttpResponseMessageThisRequest(DownstreamContext context, HttpResponseMessage response) diff --git a/src/Ocelot/Claims/Middleware/ClaimsBuilderMiddleware.cs b/src/Ocelot/Claims/Middleware/ClaimsBuilderMiddleware.cs index 48f57834..2dfc3dc0 100644 --- a/src/Ocelot/Claims/Middleware/ClaimsBuilderMiddleware.cs +++ b/src/Ocelot/Claims/Middleware/ClaimsBuilderMiddleware.cs @@ -12,28 +12,27 @@ namespace Ocelot.Claims.Middleware { private readonly OcelotRequestDelegate _next; private readonly IAddClaimsToRequest _addClaimsToRequest; - private readonly IOcelotLogger _logger; public ClaimsBuilderMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IAddClaimsToRequest addClaimsToRequest) + :base(loggerFactory.CreateLogger()) { _next = next; _addClaimsToRequest = addClaimsToRequest; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) { if (context.DownstreamReRoute.ClaimsToClaims.Any()) { - _logger.LogDebug("this route has instructions to convert claims to other claims"); + Logger.LogDebug("this route has instructions to convert claims to other claims"); var result = _addClaimsToRequest.SetClaimsOnContext(context.DownstreamReRoute.ClaimsToClaims, context.HttpContext); if (result.IsError) { - _logger.LogDebug("error converting claims to other claims, setting pipeline error"); + Logger.LogDebug("error converting claims to other claims, setting pipeline error"); SetPipelineError(context, result.Errors); return; diff --git a/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs b/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs index 985abae4..0be61658 100644 --- a/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs +++ b/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs @@ -26,8 +26,7 @@ namespace Ocelot.Configuration.Creator if (claimToThing.IsError) { - _logger.LogDebug("ClaimsToThingCreator.BuildAddThingsToRequest", - $"Unable to extract configuration for key: {input.Key} and value: {input.Value} your configuration file is incorrect"); + _logger.LogDebug($"Unable to extract configuration for key: {input.Key} and value: {input.Value} your configuration file is incorrect"); } else { diff --git a/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs b/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs index e976e7b9..7bfc6d8e 100644 --- a/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs +++ b/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs @@ -32,7 +32,7 @@ namespace Ocelot.Configuration.Creator } else { - _logger.LogError($"Unable to add UpstreamHeaderTransform {input.Key}: {input.Value}"); + _logger.LogWarning($"Unable to add UpstreamHeaderTransform {input.Key}: {input.Value}"); } } @@ -50,7 +50,7 @@ namespace Ocelot.Configuration.Creator } else { - _logger.LogError($"Unable to add DownstreamHeaderTransform {input.Key}: {input.Value}"); + _logger.LogWarning($"Unable to add DownstreamHeaderTransform {input.Key}: {input.Value}"); } } else diff --git a/src/Ocelot/Configuration/IOcelotConfiguration.cs b/src/Ocelot/Configuration/IOcelotConfiguration.cs index fd808ae4..2353cb85 100644 --- a/src/Ocelot/Configuration/IOcelotConfiguration.cs +++ b/src/Ocelot/Configuration/IOcelotConfiguration.cs @@ -9,4 +9,4 @@ namespace Ocelot.Configuration ServiceProviderConfiguration ServiceProviderConfiguration {get;} string RequestId {get;} } -} \ No newline at end of file +} diff --git a/src/Ocelot/Configuration/Parser/ClaimToThingConfigurationParser.cs b/src/Ocelot/Configuration/Parser/ClaimToThingConfigurationParser.cs index 4d698e96..94df168f 100644 --- a/src/Ocelot/Configuration/Parser/ClaimToThingConfigurationParser.cs +++ b/src/Ocelot/Configuration/Parser/ClaimToThingConfigurationParser.cs @@ -20,22 +20,14 @@ namespace Ocelot.Configuration.Parser if (instructions.Length <= 1) { - return new ErrorResponse( - new List - { - new NoInstructionsError(SplitToken) - }); + return new ErrorResponse(new NoInstructionsError(SplitToken)); } var claimMatch = _claimRegex.IsMatch(instructions[0]); if (!claimMatch) { - return new ErrorResponse( - new List - { - new InstructionNotForClaimsError() - }); + return new ErrorResponse(new InstructionNotForClaimsError()); } var newKey = GetIndexValue(instructions[0]); @@ -53,11 +45,7 @@ namespace Ocelot.Configuration.Parser } catch (Exception exception) { - return new ErrorResponse( - new List - { - new ParsingConfigurationHeaderError(exception) - }); + return new ErrorResponse(new ParsingConfigurationHeaderError(exception)); } } diff --git a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPoller.cs b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPoller.cs index 79efddca..16108d15 100644 --- a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPoller.cs +++ b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPoller.cs @@ -45,13 +45,13 @@ namespace Ocelot.Configuration.Repository private async Task Poll() { - _logger.LogDebug("Started polling consul"); + _logger.LogInformation("Started polling consul"); var fileConfig = await _repo.Get(); if(fileConfig.IsError) { - _logger.LogDebug($"error geting file config, errors are {string.Join(",", fileConfig.Errors.Select(x => x.Message))}"); + _logger.LogWarning($"error geting file config, errors are {string.Join(",", fileConfig.Errors.Select(x => x.Message))}"); return; } @@ -63,7 +63,7 @@ namespace Ocelot.Configuration.Repository _previousAsJson = asJson; } - _logger.LogDebug("Finished polling consul"); + _logger.LogInformation("Finished polling consul"); } /// diff --git a/src/Ocelot/Configuration/Validator/ConfigurationValidationResult.cs b/src/Ocelot/Configuration/Validator/ConfigurationValidationResult.cs index 4fe3567d..7e06cb6b 100644 --- a/src/Ocelot/Configuration/Validator/ConfigurationValidationResult.cs +++ b/src/Ocelot/Configuration/Validator/ConfigurationValidationResult.cs @@ -17,8 +17,8 @@ namespace Ocelot.Configuration.Validator Errors = errors; } - public bool IsError { get; private set; } + public bool IsError { get; } - public List Errors { get; private set; } + public List Errors { get; } } } diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index bcfde4b0..35b6d92d 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -44,10 +44,7 @@ namespace Ocelot.DownstreamRouteFinder.Finder return notNullOption != null ? new OkResponse(notNullOption) : new OkResponse(nullOption); } - return new ErrorResponse(new List - { - new UnableToFindDownstreamRouteError() - }); + return new ErrorResponse(new UnableToFindDownstreamRouteError(path, httpMethod)); } private bool RouteIsApplicableToThisRequest(ReRoute reRoute, string httpMethod, string upstreamHost) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/UnableToFindDownstreamRouteError.cs b/src/Ocelot/DownstreamRouteFinder/Finder/UnableToFindDownstreamRouteError.cs index 66f7172a..81988ba5 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/UnableToFindDownstreamRouteError.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/UnableToFindDownstreamRouteError.cs @@ -4,7 +4,8 @@ namespace Ocelot.DownstreamRouteFinder.Finder { public class UnableToFindDownstreamRouteError : Error { - public UnableToFindDownstreamRouteError() : base("UnableToFindDownstreamRouteError", OcelotErrorCode.UnableToFindDownstreamRouteError) + public UnableToFindDownstreamRouteError(string path, string httpVerb) + : base($"Unable to find downstream route for path: {path}, verb: {httpVerb}", OcelotErrorCode.UnableToFindDownstreamRouteError) { } } diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index 0639135d..70e4a4e2 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -14,7 +14,6 @@ namespace Ocelot.DownstreamRouteFinder.Middleware { private readonly OcelotRequestDelegate _next; private readonly IDownstreamRouteFinder _downstreamRouteFinder; - private readonly IOcelotLogger _logger; private readonly IOcelotConfigurationProvider _configProvider; private readonly IMultiplexer _multiplexer; @@ -23,12 +22,12 @@ namespace Ocelot.DownstreamRouteFinder.Middleware IDownstreamRouteFinder downstreamRouteFinder, IOcelotConfigurationProvider configProvider, IMultiplexer multiplexer) + :base(loggerFactory.CreateLogger()) { _configProvider = configProvider; _multiplexer = multiplexer; _next = next; _downstreamRouteFinder = downstreamRouteFinder; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -41,27 +40,27 @@ namespace Ocelot.DownstreamRouteFinder.Middleware if (configuration.IsError) { - _logger.LogError($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); + Logger.LogWarning($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); SetPipelineError(context, configuration.Errors); return; } context.ServiceProviderConfiguration = configuration.Data.ServiceProviderConfiguration; - _logger.LogDebug("upstream url path is {upstreamUrlPath}", upstreamUrlPath); + Logger.LogDebug($"Upstream url path is {upstreamUrlPath}"); var downstreamRoute = _downstreamRouteFinder.FindDownstreamRoute(upstreamUrlPath, context.HttpContext.Request.Method, configuration.Data, upstreamHost); if (downstreamRoute.IsError) { - _logger.LogError($"{MiddlewareName} setting pipeline errors. IDownstreamRouteFinder returned {downstreamRoute.Errors.ToErrorString()}"); + Logger.LogWarning($"{MiddlewareName} setting pipeline errors. IDownstreamRouteFinder returned {downstreamRoute.Errors.ToErrorString()}"); SetPipelineError(context, downstreamRoute.Errors); return; } var downstreamPathTemplates = string.Join(", ", downstreamRoute.Data.ReRoute.DownstreamReRoute.Select(r => r.DownstreamPathTemplate.Value)); - _logger.LogDebug($"downstream templates are {downstreamPathTemplates}"); + Logger.LogDebug($"downstream templates are {downstreamPathTemplates}"); context.TemplatePlaceholderNameAndValues = downstreamRoute.Data.TemplatePlaceholderNameAndValues; diff --git a/src/Ocelot/DownstreamUrlCreator/IUrlBuilder.cs b/src/Ocelot/DownstreamUrlCreator/IUrlBuilder.cs deleted file mode 100644 index 9975cec2..00000000 --- a/src/Ocelot/DownstreamUrlCreator/IUrlBuilder.cs +++ /dev/null @@ -1,12 +0,0 @@ -/* -using Ocelot.Responses; -using Ocelot.Values; - -namespace Ocelot.DownstreamUrlCreator -{ - public interface IUrlBuilder - { - Response Build(string downstreamPath, string downstreamScheme, ServiceHostAndPort downstreamHostAndPort); - } -} -*/ diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 43944050..bc054783 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -16,15 +16,14 @@ namespace Ocelot.DownstreamUrlCreator.Middleware { private readonly OcelotRequestDelegate _next; private readonly IDownstreamPathPlaceholderReplacer _replacer; - private readonly IOcelotLogger _logger; public DownstreamUrlCreatorMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IDownstreamPathPlaceholderReplacer replacer) + :base(loggerFactory.CreateLogger()) { _next = next; _replacer = replacer; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -34,7 +33,7 @@ namespace Ocelot.DownstreamUrlCreator.Middleware if (dsPath.IsError) { - _logger.LogDebug("IDownstreamPathPlaceholderReplacer returned an error, setting pipeline error"); + Logger.LogDebug("IDownstreamPathPlaceholderReplacer returned an error, setting pipeline error"); SetPipelineError(context, dsPath.Errors); return; @@ -53,7 +52,7 @@ namespace Ocelot.DownstreamUrlCreator.Middleware context.DownstreamRequest.AbsolutePath = dsPath.Data.Value; } - _logger.LogDebug("downstream url is {context.DownstreamRequest}", context.DownstreamRequest); + Logger.LogDebug($"Downstream url is {context.DownstreamRequest}"); await _next.Invoke(context); } diff --git a/src/Ocelot/DownstreamUrlCreator/UrlBuilder.cs b/src/Ocelot/DownstreamUrlCreator/UrlBuilder.cs deleted file mode 100644 index 1c5f284c..00000000 --- a/src/Ocelot/DownstreamUrlCreator/UrlBuilder.cs +++ /dev/null @@ -1,47 +0,0 @@ -/* -using System; -using System.Collections.Generic; -using Ocelot.Errors; -using Ocelot.Responses; -using Ocelot.Values; - -namespace Ocelot.DownstreamUrlCreator -{ - public class UrlBuilder : IUrlBuilder - { - public Response Build(string downstreamPath, string downstreamScheme, ServiceHostAndPort downstreamHostAndPort) - { - if (string.IsNullOrEmpty(downstreamPath)) - { - return new ErrorResponse(new List {new DownstreamPathNullOrEmptyError()}); - } - - if (string.IsNullOrEmpty(downstreamScheme)) - { - return new ErrorResponse(new List { new DownstreamSchemeNullOrEmptyError() }); - } - - if (string.IsNullOrEmpty(downstreamHostAndPort.DownstreamHost)) - { - return new ErrorResponse(new List { new DownstreamHostNullOrEmptyError() }); - } - - var builder = new UriBuilder - { - Host = downstreamHostAndPort.DownstreamHost, - Path = downstreamPath, - Scheme = downstreamScheme, - }; - - if (downstreamHostAndPort.DownstreamPort > 0) - { - builder.Port = downstreamHostAndPort.DownstreamPort; - } - - var url = builder.Uri.ToString(); - - return new OkResponse(new DownstreamUrl(url)); - } - } -} -*/ diff --git a/src/Ocelot/Errors/Error.cs b/src/Ocelot/Errors/Error.cs index 520f3775..e063934b 100644 --- a/src/Ocelot/Errors/Error.cs +++ b/src/Ocelot/Errors/Error.cs @@ -16,4 +16,4 @@ namespace Ocelot.Errors return Message; } } -} \ No newline at end of file +} diff --git a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs index 2b9e22e2..236e2e8d 100644 --- a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs +++ b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs @@ -18,7 +18,6 @@ namespace Ocelot.Errors.Middleware public class ExceptionHandlerMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly IOcelotConfigurationProvider _provider; private readonly IRequestScopedDataRepository _repo; @@ -26,11 +25,11 @@ namespace Ocelot.Errors.Middleware IOcelotLoggerFactory loggerFactory, IOcelotConfigurationProvider provider, IRequestScopedDataRepository repo) + : base(loggerFactory.CreateLogger()) { _provider = provider; _repo = repo; _next = next; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -39,47 +38,44 @@ namespace Ocelot.Errors.Middleware { await TrySetGlobalRequestId(context); - _logger.LogDebug("ocelot pipeline started"); + Logger.LogDebug("ocelot pipeline started"); await _next.Invoke(context); } catch (Exception e) { - _logger.LogDebug("error calling middleware"); + Logger.LogDebug("error calling middleware"); var message = CreateMessage(context, e); - _logger.LogError(message, e); + Logger.LogError(message, e); SetInternalServerErrorOnResponse(context); } - _logger.LogDebug("ocelot pipeline finished"); + Logger.LogDebug("ocelot pipeline finished"); } private async Task TrySetGlobalRequestId(DownstreamContext context) { - //try and get the global request id and set it for logs... - //should this basically be immutable per request...i guess it should! - //first thing is get config - var configuration = await _provider.Get(); + //try and get the global request id and set it for logs... + //should this basically be immutable per request...i guess it should! + //first thing is get config + var configuration = await _provider.Get(); - //if error throw to catch below.. - if(configuration.IsError) - { - throw new Exception($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); - } - - //else set the request id? - var key = configuration.Data.RequestId; - - StringValues upstreamRequestIds; - if (!string.IsNullOrEmpty(key) && context.HttpContext.Request.Headers.TryGetValue(key, out upstreamRequestIds)) - { - //todo fix looking in both places - context.HttpContext.TraceIdentifier = upstreamRequestIds.First(); - _repo.Add("RequestId", context.HttpContext.TraceIdentifier); + if(configuration.IsError) + { + throw new Exception($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); } + + var key = configuration.Data.RequestId; + + if (!string.IsNullOrEmpty(key) && context.HttpContext.Request.Headers.TryGetValue(key, out var upstreamRequestIds)) + { + context.HttpContext.TraceIdentifier = upstreamRequestIds.First(); + } + + _repo.Add("RequestId", context.HttpContext.TraceIdentifier); } private void SetInternalServerErrorOnResponse(DownstreamContext context) diff --git a/src/Ocelot/Headers/AddHeadersToResponse.cs b/src/Ocelot/Headers/AddHeadersToResponse.cs index 40d829ee..76e9912b 100644 --- a/src/Ocelot/Headers/AddHeadersToResponse.cs +++ b/src/Ocelot/Headers/AddHeadersToResponse.cs @@ -27,7 +27,7 @@ namespace Ocelot.Headers if(value.IsError) { - _logger.LogError($"Unable to add header to response {add.Key}: {add.Value}"); + _logger.LogWarning($"Unable to add header to response {add.Key}: {add.Value}"); continue; } diff --git a/src/Ocelot/Headers/Middleware/HttpHeadersTransformationMiddleware.cs b/src/Ocelot/Headers/Middleware/HttpHeadersTransformationMiddleware.cs index b09b40f8..de5c25da 100644 --- a/src/Ocelot/Headers/Middleware/HttpHeadersTransformationMiddleware.cs +++ b/src/Ocelot/Headers/Middleware/HttpHeadersTransformationMiddleware.cs @@ -10,7 +10,6 @@ namespace Ocelot.Headers.Middleware public class HttpHeadersTransformationMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly IHttpContextRequestHeaderReplacer _preReplacer; private readonly IHttpResponseHeaderReplacer _postReplacer; private readonly IAddHeadersToResponse _addHeaders; @@ -20,12 +19,12 @@ namespace Ocelot.Headers.Middleware IHttpContextRequestHeaderReplacer preReplacer, IHttpResponseHeaderReplacer postReplacer, IAddHeadersToResponse addHeaders) + :base(loggerFactory.CreateLogger()) { _addHeaders = addHeaders; _next = next; _postReplacer = postReplacer; _preReplacer = preReplacer; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) diff --git a/src/Ocelot/Headers/Middleware/HttpRequestHeadersBuilderMiddleware.cs b/src/Ocelot/Headers/Middleware/HttpRequestHeadersBuilderMiddleware.cs index 6e8dc31e..ba340b3a 100644 --- a/src/Ocelot/Headers/Middleware/HttpRequestHeadersBuilderMiddleware.cs +++ b/src/Ocelot/Headers/Middleware/HttpRequestHeadersBuilderMiddleware.cs @@ -12,34 +12,33 @@ namespace Ocelot.Headers.Middleware { private readonly OcelotRequestDelegate _next; private readonly IAddHeadersToRequest _addHeadersToRequest; - private readonly IOcelotLogger _logger; public HttpRequestHeadersBuilderMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, - IAddHeadersToRequest addHeadersToRequest) + IAddHeadersToRequest addHeadersToRequest) + :base(loggerFactory.CreateLogger()) { _next = next; _addHeadersToRequest = addHeadersToRequest; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) { if (context.DownstreamReRoute.ClaimsToHeaders.Any()) { - _logger.LogDebug($"{ context.DownstreamReRoute.DownstreamPathTemplate.Value} has instructions to convert claims to headers"); + Logger.LogInformation($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} has instructions to convert claims to headers"); var response = _addHeadersToRequest.SetHeadersOnDownstreamRequest(context.DownstreamReRoute.ClaimsToHeaders, context.HttpContext.User.Claims, context.DownstreamRequest); if (response.IsError) { - _logger.LogDebug("Error setting headers on context, setting pipeline error"); + Logger.LogWarning("Error setting headers on context, setting pipeline error"); SetPipelineError(context, response.Errors); return; } - _logger.LogDebug("headers have been set on context"); + Logger.LogInformation("headers have been set on context"); } await _next.Invoke(context); diff --git a/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs b/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs index 1af0acbc..773b4375 100644 --- a/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs +++ b/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs @@ -26,10 +26,7 @@ if (splits.Length < index || index < 0) { - return new ErrorResponse(new List - { - new CannotFindClaimError($"Cannot find claim for key: {key}, delimiter: {delimiter}, index: {index}") - }); + return new ErrorResponse(new CannotFindClaimError($"Cannot find claim for key: {key}, delimiter: {delimiter}, index: {index}")); } var value = splits[index]; @@ -55,10 +52,7 @@ return new OkResponse(claim.Value); } - return new ErrorResponse(new List - { - new CannotFindClaimError($"Cannot find claim for key: {key}") - }); + return new ErrorResponse(new CannotFindClaimError($"Cannot find claim for key: {key}")); } } } diff --git a/src/Ocelot/Infrastructure/Extensions/StringValuesExtensions.cs b/src/Ocelot/Infrastructure/Extensions/StringValuesExtensions.cs index 1b6d1682..3c85ce5c 100644 --- a/src/Ocelot/Infrastructure/Extensions/StringValuesExtensions.cs +++ b/src/Ocelot/Infrastructure/Extensions/StringValuesExtensions.cs @@ -3,7 +3,7 @@ using System.Linq; namespace Ocelot.Infrastructure.Extensions { - internal static class StringValueExtensions + internal static class StringValuesExtensions { public static string GetValue(this StringValues stringValues) { @@ -11,6 +11,7 @@ namespace Ocelot.Infrastructure.Extensions { return stringValues; } + return stringValues.ToArray().LastOrDefault(); } } diff --git a/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs b/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs index 0f75411d..ec2ce4e0 100644 --- a/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs +++ b/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs @@ -24,10 +24,7 @@ namespace Ocelot.Infrastructure.RequestData } catch (Exception exception) { - return new ErrorResponse(new List - { - new CannotAddDataError(string.Format($"Unable to add data for key: {key}, exception: {exception.Message}")) - }); + return new ErrorResponse(new CannotAddDataError(string.Format($"Unable to add data for key: {key}, exception: {exception.Message}"))); } } @@ -40,10 +37,7 @@ namespace Ocelot.Infrastructure.RequestData } catch (Exception exception) { - return new ErrorResponse(new List - { - new CannotAddDataError(string.Format($"Unable to update data for key: {key}, exception: {exception.Message}")) - }); + return new ErrorResponse(new CannotAddDataError(string.Format($"Unable to update data for key: {key}, exception: {exception.Message}"))); } } @@ -53,10 +47,7 @@ namespace Ocelot.Infrastructure.RequestData if(_httpContextAccessor.HttpContext == null || _httpContextAccessor.HttpContext.Items == null) { - return new ErrorResponse(new List - { - new CannotFindDataError($"Unable to find data for key: {key} because HttpContext or HttpContext.Items is null") - }); + return new ErrorResponse(new CannotFindDataError($"Unable to find data for key: {key} because HttpContext or HttpContext.Items is null")); } if(_httpContextAccessor.HttpContext.Items.TryGetValue(key, out obj)) @@ -65,10 +56,7 @@ namespace Ocelot.Infrastructure.RequestData return new OkResponse(data); } - return new ErrorResponse(new List - { - new CannotFindDataError($"Unable to find data for key: {key}") - }); + return new ErrorResponse(new CannotFindDataError($"Unable to find data for key: {key}")); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs index 1c99c3c5..2810e021 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs @@ -28,12 +28,12 @@ namespace Ocelot.LoadBalancer.LoadBalancers if (services == null) { - return new ErrorResponse(new List() { new ServicesAreNullError($"services were null for {_serviceName}") }); + return new ErrorResponse(new ServicesAreNullError($"services were null for {_serviceName}") ); } if (!services.Any()) { - return new ErrorResponse(new List() { new ServicesAreEmptyError($"services were empty for {_serviceName}") }); + return new ErrorResponse(new ServicesAreEmptyError($"services were empty for {_serviceName}")); } lock(_syncLock) diff --git a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs index 82f792f9..f5ddf6f2 100644 --- a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs +++ b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs @@ -9,15 +9,14 @@ namespace Ocelot.LoadBalancer.Middleware public class LoadBalancingMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly ILoadBalancerHouse _loadBalancerHouse; public LoadBalancingMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, ILoadBalancerHouse loadBalancerHouse) + :base(loggerFactory.CreateLogger()) { _next = next; - _logger = loggerFactory.CreateLogger(); _loadBalancerHouse = loadBalancerHouse; } @@ -26,7 +25,7 @@ namespace Ocelot.LoadBalancer.Middleware var loadBalancer = await _loadBalancerHouse.Get(context.DownstreamReRoute, context.ServiceProviderConfiguration); if(loadBalancer.IsError) { - _logger.LogDebug("there was an error retriving the loadbalancer, setting pipeline error"); + Logger.LogDebug("there was an error retriving the loadbalancer, setting pipeline error"); SetPipelineError(context, loadBalancer.Errors); return; } @@ -34,7 +33,7 @@ namespace Ocelot.LoadBalancer.Middleware var hostAndPort = await loadBalancer.Data.Lease(); if(hostAndPort.IsError) { - _logger.LogDebug("there was an error leasing the loadbalancer, setting pipeline error"); + Logger.LogDebug("there was an error leasing the loadbalancer, setting pipeline error"); SetPipelineError(context, hostAndPort.Errors); return; } @@ -52,7 +51,7 @@ namespace Ocelot.LoadBalancer.Middleware } catch (Exception) { - _logger.LogDebug("Exception calling next middleware, exception will be thrown to global handler"); + Logger.LogDebug("Exception calling next middleware, exception will be thrown to global handler"); throw; } finally diff --git a/src/Ocelot/Logging/AspDotNetLogger.cs b/src/Ocelot/Logging/AspDotNetLogger.cs index eae1eec5..5ee04e13 100644 --- a/src/Ocelot/Logging/AspDotNetLogger.cs +++ b/src/Ocelot/Logging/AspDotNetLogger.cs @@ -1,6 +1,5 @@ using System; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Internal; using Ocelot.Infrastructure.RequestData; namespace Ocelot.Logging @@ -10,34 +9,38 @@ namespace Ocelot.Logging private readonly ILogger _logger; private readonly IRequestScopedDataRepository _scopedDataRepository; - public string Name { get; } - - public AspDotNetLogger(ILogger logger, IRequestScopedDataRepository scopedDataRepository, string typeName) + public AspDotNetLogger(ILogger logger, IRequestScopedDataRepository scopedDataRepository) { - Name = typeName; _logger = logger; _scopedDataRepository = scopedDataRepository; } - public void LogTrace(string message, params object[] args) + public void LogTrace(string message) { var requestId = GetOcelotRequestId(); var previousRequestId = GetOcelotPreviousRequestId(); - _logger.LogTrace("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message},", requestId, previousRequestId, new FormattedLogValues(message, args).ToString()); + _logger.LogTrace("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, message); } - public void LogDebug(string message, params object[] args) + public void LogDebug(string message) { var requestId = GetOcelotRequestId(); var previousRequestId = GetOcelotPreviousRequestId(); - _logger.LogDebug("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message},", requestId, previousRequestId, new FormattedLogValues(message, args).ToString()); + _logger.LogDebug("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, message); } - public void LogInformation(string message, params object[] args) + public void LogInformation(string message) { var requestId = GetOcelotRequestId(); var previousRequestId = GetOcelotPreviousRequestId(); - _logger.LogInformation("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message},", requestId, previousRequestId, new FormattedLogValues(message, args).ToString()); + _logger.LogInformation("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, message); + } + + public void LogWarning(string message) + { + var requestId = GetOcelotRequestId(); + var previousRequestId = GetOcelotPreviousRequestId(); + _logger.LogWarning("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, message); } public void LogError(string message, Exception exception) @@ -47,18 +50,11 @@ namespace Ocelot.Logging _logger.LogError("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}, exception: {exception}", requestId, previousRequestId, message, exception); } - public void LogError(string message, params object[] args) - { - var requestId = GetOcelotRequestId(); - var previousRequestId = GetOcelotPreviousRequestId(); - _logger.LogError("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, new FormattedLogValues(message, args).ToString()); - } - public void LogCritical(string message, Exception exception) { var requestId = GetOcelotRequestId(); var previousRequestId = GetOcelotPreviousRequestId(); - _logger.LogError("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}", requestId, previousRequestId, message); + _logger.LogCritical("requestId: {requestId}, previousRequestId: {previousRequestId}, message: {message}, exception: {exception}", requestId, previousRequestId, message, exception); } private string GetOcelotRequestId() @@ -85,4 +81,4 @@ namespace Ocelot.Logging return requestId.Data; } } -} \ No newline at end of file +} diff --git a/src/Ocelot/Logging/AspDotNetLoggerFactory.cs b/src/Ocelot/Logging/AspDotNetLoggerFactory.cs index 298653bc..b988e09c 100644 --- a/src/Ocelot/Logging/AspDotNetLoggerFactory.cs +++ b/src/Ocelot/Logging/AspDotNetLoggerFactory.cs @@ -1,8 +1,8 @@ -using Microsoft.Extensions.Logging; -using Ocelot.Infrastructure.RequestData; - -namespace Ocelot.Logging -{ +using Microsoft.Extensions.Logging; +using Ocelot.Infrastructure.RequestData; + +namespace Ocelot.Logging +{ public class AspDotNetLoggerFactory : IOcelotLoggerFactory { private readonly ILoggerFactory _loggerFactory; @@ -17,7 +17,7 @@ namespace Ocelot.Logging public IOcelotLogger CreateLogger() { var logger = _loggerFactory.CreateLogger(); - return new AspDotNetLogger(logger, _scopedDataRepository, typeof(T).Name); + return new AspDotNetLogger(logger, _scopedDataRepository); } - } + } } \ No newline at end of file diff --git a/src/Ocelot/Logging/IOcelotLogger.cs b/src/Ocelot/Logging/IOcelotLogger.cs index 67bac731..7d1c1205 100644 --- a/src/Ocelot/Logging/IOcelotLogger.cs +++ b/src/Ocelot/Logging/IOcelotLogger.cs @@ -7,16 +7,11 @@ namespace Ocelot.Logging /// public interface IOcelotLogger { - void LogTrace(string message, params object[] args); - void LogDebug(string message, params object[] args); - void LogInformation(string message, params object[] args); + void LogTrace(string message); + void LogDebug(string message); + void LogInformation(string message); + void LogWarning(string message); void LogError(string message, Exception exception); - void LogError(string message, params object[] args); void LogCritical(string message, Exception exception); - - /// - /// The name of the type the logger has been built for. - /// - string Name { get; } } } diff --git a/src/Ocelot/Logging/OcelotDiagnosticListener.cs b/src/Ocelot/Logging/OcelotDiagnosticListener.cs index d75cfd2c..f2c51760 100644 --- a/src/Ocelot/Logging/OcelotDiagnosticListener.cs +++ b/src/Ocelot/Logging/OcelotDiagnosticListener.cs @@ -8,15 +8,14 @@ using Butterfly.Client.Tracing; using System.Linq; using System.Collections.Generic; using Ocelot.Infrastructure.Extensions; -using Microsoft.Extensions.Logging; using Ocelot.Requester; namespace Ocelot.Logging { public class OcelotDiagnosticListener { - private IServiceTracer _tracer; - private IOcelotLogger _logger; + private readonly IServiceTracer _tracer; + private readonly IOcelotLogger _logger; public OcelotDiagnosticListener(IOcelotLoggerFactory factory, IServiceTracer tracer) { @@ -27,7 +26,7 @@ namespace Ocelot.Logging [DiagnosticName("Ocelot.MiddlewareException")] public virtual void OcelotMiddlewareException(Exception exception, DownstreamContext context, string name) { - _logger.LogTrace($"Ocelot.MiddlewareException: {name}; {exception.Message}"); + _logger.LogTrace($"Ocelot.MiddlewareException: {name}; {exception.Message};"); Event(context.HttpContext, $"Ocelot.MiddlewareStarted: {name}; {context.HttpContext.Request.Path}"); } @@ -41,7 +40,7 @@ namespace Ocelot.Logging [DiagnosticName("Ocelot.MiddlewareFinished")] public virtual void OcelotMiddlewareFinished(DownstreamContext context, string name) { - _logger.LogTrace($"OcelotMiddlewareFinished: {name}; {context.HttpContext.Request.Path}"); + _logger.LogTrace($"Ocelot.MiddlewareFinished: {name}; {context.HttpContext.Request.Path}"); Event(context.HttpContext, $"OcelotMiddlewareFinished: {name}; {context.HttpContext.Request.Path}"); } @@ -55,7 +54,7 @@ namespace Ocelot.Logging [DiagnosticName("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareException")] public virtual void OnMiddlewareException(Exception exception, string name) { - _logger.LogTrace($"MiddlewareException: {name}; {exception.Message}"); + _logger.LogTrace($"MiddlewareException: {name}; {exception.Message};"); } [DiagnosticName("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareFinished")] @@ -67,16 +66,17 @@ namespace Ocelot.Logging private void Event(HttpContext httpContext, string @event) { - // Hack - if the user isnt using tracing the code gets here and will blow up on + // todo - if the user isnt using tracing the code gets here and will blow up on // _tracer.Tracer.TryExtract. We already use the fake tracer for another scenario - // so sticking it here as well..I guess we need a factory for this but no idea - // how to hook that into the diagnostic framework at the moment. + // so sticking it here as well..I guess we need a factory for this but cba to do it at + // the moment if(_tracer.GetType() == typeof(FakeServiceTracer)) { return; } var span = httpContext.GetSpan(); + if(span == null) { var spanBuilder = new SpanBuilder($"server {httpContext.Request.Method} {httpContext.Request.Path}"); @@ -84,10 +84,12 @@ namespace Ocelot.Logging c => c.Select(x => new KeyValuePair(x.Key, x.Value.GetValue())).GetEnumerator())) { spanBuilder.AsChildOf(spanContext); - }; + } + span = _tracer.Start(spanBuilder); httpContext.SetSpan(span); } + span?.Log(LogField.CreateNew().Event(@event)); } } diff --git a/src/Ocelot/Middleware/DownstreamContext.cs b/src/Ocelot/Middleware/DownstreamContext.cs index 9e054eb5..044d1510 100644 --- a/src/Ocelot/Middleware/DownstreamContext.cs +++ b/src/Ocelot/Middleware/DownstreamContext.cs @@ -13,17 +13,24 @@ namespace Ocelot.Middleware { public DownstreamContext(HttpContext httpContext) { - this.HttpContext = httpContext; + HttpContext = httpContext; Errors = new List(); } public List TemplatePlaceholderNameAndValues { get; set; } + public ServiceProviderConfiguration ServiceProviderConfiguration {get; set;} - public HttpContext HttpContext { get; private set; } + + public HttpContext HttpContext { get; } + public DownstreamReRoute DownstreamReRoute { get; set; } + public DownstreamRequest DownstreamRequest { get; set; } + public HttpResponseMessage DownstreamResponse { get; set; } + public List Errors { get;set; } + public bool IsError => Errors.Count > 0; } } diff --git a/src/Ocelot/Middleware/OcelotMiddleware.cs b/src/Ocelot/Middleware/OcelotMiddleware.cs index bf49fd03..fe51fd24 100644 --- a/src/Ocelot/Middleware/OcelotMiddleware.cs +++ b/src/Ocelot/Middleware/OcelotMiddleware.cs @@ -1,20 +1,33 @@ using System.Collections.Generic; using Ocelot.Errors; +using Ocelot.Logging; namespace Ocelot.Middleware { public abstract class OcelotMiddleware - { - protected OcelotMiddleware() + { + protected OcelotMiddleware(IOcelotLogger logger) { + Logger = logger; MiddlewareName = this.GetType().Name; } + public IOcelotLogger Logger { get; } + public string MiddlewareName { get; } public void SetPipelineError(DownstreamContext context, List errors) { - context.Errors = errors; + foreach(var error in errors) + { + SetPipelineError(context, error); + } + } + + public void SetPipelineError(DownstreamContext context, Error error) + { + Logger.LogWarning(error.Message); + context.Errors.Add(error); } } } diff --git a/src/Ocelot/Middleware/Pipeline/OcelotPipelineBuilderExtensions.cs b/src/Ocelot/Middleware/Pipeline/OcelotPipelineBuilderExtensions.cs index 539fa6a1..573e0924 100644 --- a/src/Ocelot/Middleware/Pipeline/OcelotPipelineBuilderExtensions.cs +++ b/src/Ocelot/Middleware/Pipeline/OcelotPipelineBuilderExtensions.cs @@ -88,7 +88,7 @@ namespace Ocelot.Middleware.Pipeline } catch(Exception ex) { - Write(diagnosticListener, "Ocelot.MiddlewareException", middlewareName, context); + WriteException(diagnosticListener, ex, "Ocelot.MiddlewareException", middlewareName, context); throw ex; } finally @@ -123,6 +123,14 @@ namespace Ocelot.Middleware.Pipeline } } + private static void WriteException(DiagnosticListener diagnosticListener, Exception exception, string message, string middlewareName, DownstreamContext context) + { + if(diagnosticListener != null) + { + diagnosticListener.Write(message, new { name = middlewareName, context = context, exception = exception }); + } + } + public static IOcelotPipelineBuilder MapWhen(this IOcelotPipelineBuilder app, Predicate predicate, Action configuration) { if (app == null) diff --git a/src/Ocelot/QueryStrings/Middleware/QueryStringBuilderMiddleware.cs b/src/Ocelot/QueryStrings/Middleware/QueryStringBuilderMiddleware.cs index ae5dca85..bc886c5d 100644 --- a/src/Ocelot/QueryStrings/Middleware/QueryStringBuilderMiddleware.cs +++ b/src/Ocelot/QueryStrings/Middleware/QueryStringBuilderMiddleware.cs @@ -12,28 +12,27 @@ namespace Ocelot.QueryStrings.Middleware { private readonly OcelotRequestDelegate _next; private readonly IAddQueriesToRequest _addQueriesToRequest; - private readonly IOcelotLogger _logger; public QueryStringBuilderMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IAddQueriesToRequest addQueriesToRequest) + : base(loggerFactory.CreateLogger()) { _next = next; _addQueriesToRequest = addQueriesToRequest; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) { if (context.DownstreamReRoute.ClaimsToQueries.Any()) { - _logger.LogDebug($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} has instructions to convert claims to queries"); + Logger.LogInformation($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} has instructions to convert claims to queries"); var response = _addQueriesToRequest.SetQueriesOnDownstreamRequest(context.DownstreamReRoute.ClaimsToQueries, context.HttpContext.User.Claims, context.DownstreamRequest); if (response.IsError) { - _logger.LogDebug("there was an error setting queries on context, setting pipeline error"); + Logger.LogWarning("there was an error setting queries on context, setting pipeline error"); SetPipelineError(context, response.Errors); return; diff --git a/src/Ocelot/Raft/RaftController.cs b/src/Ocelot/Raft/RaftController.cs index eb91e86f..c1222e4d 100644 --- a/src/Ocelot/Raft/RaftController.cs +++ b/src/Ocelot/Raft/RaftController.cs @@ -40,9 +40,13 @@ namespace Ocelot.Raft using(var reader = new StreamReader(HttpContext.Request.Body)) { var json = await reader.ReadToEndAsync(); + var appendEntries = JsonConvert.DeserializeObject(json, _jsonSerialiserSettings); + _logger.LogDebug($"{_baseSchemeUrlAndPort}/appendentries called, my state is {_node.State.GetType().FullName}"); + var appendEntriesResponse = _node.Handle(appendEntries); + return new OkObjectResult(appendEntriesResponse); } } @@ -53,9 +57,13 @@ namespace Ocelot.Raft using(var reader = new StreamReader(HttpContext.Request.Body)) { var json = await reader.ReadToEndAsync(); + var requestVote = JsonConvert.DeserializeObject(json, _jsonSerialiserSettings); + _logger.LogDebug($"{_baseSchemeUrlAndPort}/requestvote called, my state is {_node.State.GetType().FullName}"); + var requestVoteResponse = _node.Handle(requestVote); + return new OkObjectResult(requestVoteResponse); } } @@ -68,10 +76,15 @@ namespace Ocelot.Raft using(var reader = new StreamReader(HttpContext.Request.Body)) { var json = await reader.ReadToEndAsync(); + var command = JsonConvert.DeserializeObject(json, _jsonSerialiserSettings); + _logger.LogDebug($"{_baseSchemeUrlAndPort}/command called, my state is {_node.State.GetType().FullName}"); + var commandResponse = _node.Accept(command); + json = JsonConvert.SerializeObject(commandResponse, _jsonSerialiserSettings); + return StatusCode(200, json); } } diff --git a/src/Ocelot/RateLimit/Middleware/ClientRateLimitMiddleware.cs b/src/Ocelot/RateLimit/Middleware/ClientRateLimitMiddleware.cs index 4ceb91f8..50eb7776 100644 --- a/src/Ocelot/RateLimit/Middleware/ClientRateLimitMiddleware.cs +++ b/src/Ocelot/RateLimit/Middleware/ClientRateLimitMiddleware.cs @@ -14,16 +14,15 @@ namespace Ocelot.RateLimit.Middleware public class ClientRateLimitMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly IRateLimitCounterHandler _counterHandler; private readonly ClientRateLimitProcessor _processor; public ClientRateLimitMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IRateLimitCounterHandler counterHandler) + :base(loggerFactory.CreateLogger()) { _next = next; - _logger = loggerFactory.CreateLogger(); _counterHandler = counterHandler; _processor = new ClientRateLimitProcessor(counterHandler); } @@ -35,7 +34,7 @@ namespace Ocelot.RateLimit.Middleware // check if rate limiting is enabled if (!context.DownstreamReRoute.EnableEndpointEndpointRateLimiting) { - _logger.LogDebug($"EndpointRateLimiting is not enabled for {context.DownstreamReRoute.DownstreamPathTemplate}"); + Logger.LogInformation($"EndpointRateLimiting is not enabled for {context.DownstreamReRoute.DownstreamPathTemplate.Value}"); await _next.Invoke(context); return; } @@ -46,7 +45,7 @@ namespace Ocelot.RateLimit.Middleware // check white list if (IsWhitelisted(identity, options)) { - _logger.LogDebug($"{context.DownstreamReRoute.DownstreamPathTemplate} is white listed from rate limiting"); + Logger.LogInformation($"{context.DownstreamReRoute.DownstreamPathTemplate.Value} is white listed from rate limiting"); await _next.Invoke(context); return; } @@ -112,9 +111,10 @@ namespace Ocelot.RateLimit.Middleware public virtual void LogBlockedRequest(HttpContext httpContext, ClientRequestIdentity identity, RateLimitCounter counter, RateLimitRule rule, DownstreamReRoute downstreamReRoute) { - _logger.LogDebug($"Request {identity.HttpVerb}:{identity.Path} from ClientId {identity.ClientId} has been blocked, quota {rule.Limit}/{rule.Period} exceeded by {counter.TotalRequests}. Blocked by rule { downstreamReRoute.UpstreamPathTemplate }, TraceIdentifier {httpContext.TraceIdentifier}."); + Logger.LogInformation( + $"Request {identity.HttpVerb}:{identity.Path} from ClientId {identity.ClientId} has been blocked, quota {rule.Limit}/{rule.Period} exceeded by {counter.TotalRequests}. Blocked by rule { downstreamReRoute.UpstreamPathTemplate.Value }, TraceIdentifier {httpContext.TraceIdentifier}."); } - + public virtual Task ReturnQuotaExceededResponse(HttpContext httpContext, RateLimitOptions option, string retryAfter) { var message = string.IsNullOrEmpty(option.QuotaExceededMessage) ? $"API calls quota exceeded! maximum admitted {option.RateLimitRule.Limit} per {option.RateLimitRule.Period}." : option.QuotaExceededMessage; diff --git a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs index ecdb134a..0cb7c7dd 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs @@ -11,15 +11,14 @@ namespace Ocelot.Request.Middleware public class DownstreamRequestInitialiserMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly Mapper.IRequestMapper _requestMapper; public DownstreamRequestInitialiserMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, Mapper.IRequestMapper requestMapper) + :base(loggerFactory.CreateLogger()) { _next = next; - _logger = loggerFactory.CreateLogger(); _requestMapper = requestMapper; } diff --git a/src/Ocelot/RequestId/Middleware/ReRouteRequestIdMiddleware.cs b/src/Ocelot/RequestId/Middleware/ReRouteRequestIdMiddleware.cs index 7ae416b0..f999bf40 100644 --- a/src/Ocelot/RequestId/Middleware/ReRouteRequestIdMiddleware.cs +++ b/src/Ocelot/RequestId/Middleware/ReRouteRequestIdMiddleware.cs @@ -1,14 +1,11 @@ +using System; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Primitives; using Ocelot.Infrastructure.RequestData; using Ocelot.Logging; using Ocelot.Middleware; -using System.Net.Http; using System.Net.Http.Headers; using System.Collections.Generic; -using Ocelot.DownstreamRouteFinder.Middleware; using Ocelot.Request.Middleware; namespace Ocelot.RequestId.Middleware @@ -16,16 +13,15 @@ namespace Ocelot.RequestId.Middleware public class ReRouteRequestIdMiddleware : OcelotMiddleware { private readonly OcelotRequestDelegate _next; - private readonly IOcelotLogger _logger; private readonly IRequestScopedDataRepository _requestScopedDataRepository; public ReRouteRequestIdMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IRequestScopedDataRepository requestScopedDataRepository) + : base(loggerFactory.CreateLogger()) { _next = next; _requestScopedDataRepository = requestScopedDataRepository; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -38,26 +34,23 @@ namespace Ocelot.RequestId.Middleware { // if get request ID is set on upstream request then retrieve it var key = context.DownstreamReRoute.RequestIdKey ?? DefaultRequestIdKey.Value; - - StringValues upstreamRequestIds; - if (context.HttpContext.Request.Headers.TryGetValue(key, out upstreamRequestIds)) + + if (context.HttpContext.Request.Headers.TryGetValue(key, out var upstreamRequestIds)) { - //set the traceidentifier context.HttpContext.TraceIdentifier = upstreamRequestIds.First(); - //todo fix looking in both places //check if we have previous id in scoped repo var previousRequestId = _requestScopedDataRepository.Get("RequestId"); - if (!previousRequestId.IsError && !string.IsNullOrEmpty(previousRequestId.Data)) + if (!previousRequestId.IsError && !string.IsNullOrEmpty(previousRequestId.Data) && previousRequestId.Data != context.HttpContext.TraceIdentifier) { //we have a previous request id lets store it and update request id - _requestScopedDataRepository.Add("PreviousRequestId", previousRequestId.Data); - _requestScopedDataRepository.Update("RequestId", context.HttpContext.TraceIdentifier); + _requestScopedDataRepository.Add("PreviousRequestId", previousRequestId.Data); + _requestScopedDataRepository.Update("RequestId", context.HttpContext.TraceIdentifier); } else { //else just add request id - _requestScopedDataRepository.Add("RequestId", context.HttpContext.TraceIdentifier); + _requestScopedDataRepository.Add("RequestId", context.HttpContext.TraceIdentifier); } } diff --git a/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs b/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs index 68397b9f..23ebd9de 100644 --- a/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs +++ b/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs @@ -12,15 +12,14 @@ namespace Ocelot.Requester.Middleware { private readonly OcelotRequestDelegate _next; private readonly IHttpRequester _requester; - private readonly IOcelotLogger _logger; public HttpRequesterMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory, IHttpRequester requester) + : base(loggerFactory.CreateLogger()) { _next = next; _requester = requester; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -29,13 +28,13 @@ namespace Ocelot.Requester.Middleware if (response.IsError) { - _logger.LogDebug("IHttpRequester returned an error, setting pipeline error"); + Logger.LogDebug("IHttpRequester returned an error, setting pipeline error"); SetPipelineError(context, response.Errors); return; } - _logger.LogDebug("setting http response message"); + Logger.LogDebug("setting http response message"); context.DownstreamResponse = response.Data; } diff --git a/src/Ocelot/Requester/OcelotHttpTracingHandler.cs b/src/Ocelot/Requester/OcelotHttpTracingHandler.cs index 9de364ac..fd6dce2b 100644 --- a/src/Ocelot/Requester/OcelotHttpTracingHandler.cs +++ b/src/Ocelot/Requester/OcelotHttpTracingHandler.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -13,7 +12,7 @@ namespace Ocelot.Requester { private readonly IServiceTracer _tracer; private readonly IRequestScopedDataRepository _repo; - private const string prefix_spanId = "ot-spanId"; + private const string PrefixSpanId = "ot-spanId"; public OcelotHttpTracingHandler( IServiceTracer tracer, @@ -37,11 +36,10 @@ namespace Ocelot.Requester HttpRequestMessage request, CancellationToken cancellationToken) { - IEnumerable traceIdVals = null; - if (request.Headers.TryGetValues(prefix_spanId, out traceIdVals)) + if (request.Headers.Contains(PrefixSpanId)) { - request.Headers.Remove(prefix_spanId); - request.Headers.TryAddWithoutValidation(prefix_spanId, span.SpanContext.SpanId); + request.Headers.Remove(PrefixSpanId); + request.Headers.TryAddWithoutValidation(PrefixSpanId, span.SpanContext.SpanId); } _repo.Add("TraceId", span.SpanContext.TraceId); diff --git a/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs b/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs index 125280b0..3e04fa3a 100644 --- a/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs +++ b/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs @@ -6,6 +6,7 @@ using Ocelot.Middleware; using System.Collections.Generic; using System.Threading.Tasks; using Ocelot.DownstreamRouteFinder.Middleware; +using Ocelot.Infrastructure.Extensions; namespace Ocelot.Responder.Middleware { @@ -17,18 +18,17 @@ namespace Ocelot.Responder.Middleware private readonly OcelotRequestDelegate _next; private readonly IHttpResponder _responder; private readonly IErrorsToHttpStatusCodeMapper _codeMapper; - private readonly IOcelotLogger _logger; public ResponderMiddleware(OcelotRequestDelegate next, IHttpResponder responder, IOcelotLoggerFactory loggerFactory, IErrorsToHttpStatusCodeMapper codeMapper ) + :base(loggerFactory.CreateLogger()) { _next = next; _responder = responder; _codeMapper = codeMapper; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) @@ -37,19 +37,13 @@ namespace Ocelot.Responder.Middleware if (context.IsError) { - var errors = context.Errors; - _logger.LogError($"{errors.Count} pipeline errors found in {MiddlewareName}. Setting error response status code"); - - foreach(var error in errors) - { - _logger.LogError(error.Message); - } + Logger.LogWarning($"{context.Errors.ToErrorString()} errors found in {MiddlewareName}. Setting error response for request path:{context.HttpContext.Request.Path}, request method: {context.HttpContext.Request.Method}"); - SetErrorResponse(context.HttpContext, errors); + SetErrorResponse(context.HttpContext, context.Errors); } else { - _logger.LogDebug("no pipeline errors, setting and returning completed response"); + Logger.LogDebug("no pipeline errors, setting and returning completed response"); await _responder.SetResponseOnHttpContext(context.HttpContext, context.DownstreamResponse); } } diff --git a/src/Ocelot/Responses/ErrorResponse.cs b/src/Ocelot/Responses/ErrorResponse.cs index 7a6688e7..156243ab 100644 --- a/src/Ocelot/Responses/ErrorResponse.cs +++ b/src/Ocelot/Responses/ErrorResponse.cs @@ -5,11 +5,13 @@ namespace Ocelot.Responses { public class ErrorResponse : Response { - public ErrorResponse(Error error) : base(new List{error}) + public ErrorResponse(Error error) + : base(new List{error}) { } - public ErrorResponse(List errors) : base(errors) + public ErrorResponse(List errors) + : base(errors) { } } diff --git a/src/Ocelot/Responses/Response.cs b/src/Ocelot/Responses/Response.cs index 21ec91b5..3a387a2b 100644 --- a/src/Ocelot/Responses/Response.cs +++ b/src/Ocelot/Responses/Response.cs @@ -15,14 +15,8 @@ namespace Ocelot.Responses Errors = errors ?? new List(); } - public List Errors { get; private set; } + public List Errors { get; } - public bool IsError - { - get - { - return Errors.Count > 0; - } - } + public bool IsError => Errors.Count > 0; } -} \ No newline at end of file +} diff --git a/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs index 775033fd..de31188e 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs @@ -47,7 +47,7 @@ namespace Ocelot.ServiceDiscovery.Providers } else { - _logger.LogError($"Unable to use service Address: {serviceEntry.Service.Address} and Port: {serviceEntry.Service.Port} as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"); + _logger.LogWarning($"Unable to use service Address: {serviceEntry.Service.Address} and Port: {serviceEntry.Service.Port} as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"); } } diff --git a/src/Ocelot/WebSockets/Middleware/WebSocketsProxyMiddleware.cs b/src/Ocelot/WebSockets/Middleware/WebSocketsProxyMiddleware.cs index 7a9e0ad0..68b2df48 100644 --- a/src/Ocelot/WebSockets/Middleware/WebSocketsProxyMiddleware.cs +++ b/src/Ocelot/WebSockets/Middleware/WebSocketsProxyMiddleware.cs @@ -11,13 +11,12 @@ namespace Ocelot.WebSockets.Middleware public class WebSocketsProxyMiddleware : OcelotMiddleware { private OcelotRequestDelegate _next; - private IOcelotLogger _logger; public WebSocketsProxyMiddleware(OcelotRequestDelegate next, IOcelotLoggerFactory loggerFactory) + :base(loggerFactory.CreateLogger()) { _next = next; - _logger = loggerFactory.CreateLogger(); } public async Task Invoke(DownstreamContext context) diff --git a/test/Ocelot.ManualTest/Program.cs b/test/Ocelot.ManualTest/Program.cs index 4e728d3d..3bab21b7 100644 --- a/test/Ocelot.ManualTest/Program.cs +++ b/test/Ocelot.ManualTest/Program.cs @@ -37,11 +37,11 @@ namespace Ocelot.ManualTest { x.WithDictionaryHandle(); }) - .AddOpenTracing(option => + /* .AddOpenTracing(option => { option.CollectorUrl = "http://localhost:9618"; option.Service = "Ocelot.ManualTest"; - }) + })*/ .AddAdministration("/administration", "secret"); }) .ConfigureLogging((hostingContext, logging) => diff --git a/test/Ocelot.ManualTest/appsettings.json b/test/Ocelot.ManualTest/appsettings.json index c10bfed6..584fc4fc 100644 --- a/test/Ocelot.ManualTest/appsettings.json +++ b/test/Ocelot.ManualTest/appsettings.json @@ -2,7 +2,7 @@ "Logging": { "IncludeScopes": false, "LogLevel": { - "Default": "Error", + "Default": "Trace", "System": "Error", "Microsoft": "Error" } diff --git a/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs index 522d853f..cda6d5de 100644 --- a/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs @@ -72,7 +72,7 @@ namespace Ocelot.UnitTests.Configuration private void ThenTheLoggerIsCalledCorrectly() { _logger - .Verify(x => x.LogDebug(It.IsAny(), It.IsAny()), Times.Once); + .Verify(x => x.LogDebug(It.IsAny()), Times.Once); } private void ThenClaimsToThingsAreReturned() diff --git a/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs index aa285e8d..67031879 100644 --- a/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs @@ -124,7 +124,7 @@ namespace Ocelot.UnitTests.Configuration private void ThenTheLoggerIsCalledCorrectly(string message) { - _logger.Verify(x => x.LogError(message), Times.Once); + _logger.Verify(x => x.LogWarning(message), Times.Once); } [Fact] diff --git a/test/Ocelot.UnitTests/Errors/ExceptionHandlerMiddlewareTests.cs b/test/Ocelot.UnitTests/Errors/ExceptionHandlerMiddlewareTests.cs index 89eaaa38..6b52a1c5 100644 --- a/test/Ocelot.UnitTests/Errors/ExceptionHandlerMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Errors/ExceptionHandlerMiddlewareTests.cs @@ -1,5 +1,3 @@ -using Ocelot.Middleware; - namespace Ocelot.UnitTests.Errors { using System; @@ -15,18 +13,18 @@ namespace Ocelot.UnitTests.Errors using Moq; using Ocelot.Configuration; using Ocelot.Errors; - using Ocelot.DownstreamRouteFinder.Middleware; using Ocelot.Infrastructure.RequestData; + using Ocelot.Middleware; public class ExceptionHandlerMiddlewareTests { - bool _shouldThrowAnException = false; - private Mock _provider; - private Mock _repo; + bool _shouldThrowAnException; + private readonly Mock _provider; + private readonly Mock _repo; private Mock _loggerFactory; private Mock _logger; - private ExceptionHandlerMiddleware _middleware; - private DownstreamContext _downstreamContext; + private readonly ExceptionHandlerMiddleware _middleware; + private readonly DownstreamContext _downstreamContext; private OcelotRequestDelegate _next; public ExceptionHandlerMiddlewareTests() @@ -59,7 +57,7 @@ namespace Ocelot.UnitTests.Errors .And(_ => GivenTheConfigurationIs(config)) .When(_ => WhenICallTheMiddleware()) .Then(_ => ThenTheResponseIsOk()) - .And(_ => TheRequestIdIsNotSet()) + .And(_ => TheAspDotnetRequestIdIsSet()) .BDDfy(); } @@ -89,7 +87,7 @@ namespace Ocelot.UnitTests.Errors } [Fact] - public void ShouldNotSetRequestId() + public void ShouldSetAspDotNetRequestId() { var config = new OcelotConfiguration(null, null, null, null); @@ -97,7 +95,7 @@ namespace Ocelot.UnitTests.Errors .And(_ => GivenTheConfigurationIs(config)) .When(_ => WhenICallTheMiddlewareWithTheRequestIdKey("requestidkey", "1234")) .Then(_ => ThenTheResponseIsOk()) - .And(_ => TheRequestIdIsNotSet()) + .And(_ => TheAspDotnetRequestIdIsSet()) .BDDfy(); } @@ -146,29 +144,19 @@ namespace Ocelot.UnitTests.Errors private void GivenTheConfigReturnsError() { - var config = new OcelotConfiguration(null, null, null, null); - - var response = new Ocelot.Responses.ErrorResponse(new FakeError()); + var response = new Responses.ErrorResponse(new FakeError()); _provider .Setup(x => x.Get()).ReturnsAsync(response); } - public class FakeError : Error - { - public FakeError() - : base("meh", OcelotErrorCode.CannotAddDataError) - { - } - } - private void TheRequestIdIsSet(string key, string value) { - _repo.Verify(x => x.Add(key, value), Times.Once); + _repo.Verify(x => x.Add(key, value), Times.Once); } private void GivenTheConfigurationIs(IOcelotConfiguration config) { - var response = new Ocelot.Responses.OkResponse(config); + var response = new Responses.OkResponse(config); _provider .Setup(x => x.Get()).ReturnsAsync(response); } @@ -193,9 +181,17 @@ namespace Ocelot.UnitTests.Errors _downstreamContext.HttpContext.Response.StatusCode.ShouldBe(500); } - private void TheRequestIdIsNotSet() + private void TheAspDotnetRequestIdIsSet() { - _repo.Verify(x => x.Add(It.IsAny(), It.IsAny()), Times.Never); + _repo.Verify(x => x.Add(It.IsAny(), It.IsAny()), Times.Once); + } + + class FakeError : Error + { + internal FakeError() + : base("meh", OcelotErrorCode.CannotAddDataError) + { + } } } } diff --git a/test/Ocelot.UnitTests/Headers/AddHeadersToResponseTests.cs b/test/Ocelot.UnitTests/Headers/AddHeadersToResponseTests.cs index a4a19eec..a8eba48e 100644 --- a/test/Ocelot.UnitTests/Headers/AddHeadersToResponseTests.cs +++ b/test/Ocelot.UnitTests/Headers/AddHeadersToResponseTests.cs @@ -106,7 +106,7 @@ namespace Ocelot.UnitTests.Headers private void ThenTheErrorIsLogged() { - _logger.Verify(x => x.LogError("Unable to add header to response Trace-Id: {TraceId}"), Times.Once); + _logger.Verify(x => x.LogWarning("Unable to add header to response Trace-Id: {TraceId}"), Times.Once); } private void ThenTheHeaderIsNotAdded(string key) @@ -145,4 +145,4 @@ namespace Ocelot.UnitTests.Headers _addHeaders = addHeaders; } } -} \ No newline at end of file +} diff --git a/test/Ocelot.UnitTests/Logging/AspDotNetLoggerTests.cs b/test/Ocelot.UnitTests/Logging/AspDotNetLoggerTests.cs new file mode 100644 index 00000000..8487cf8d --- /dev/null +++ b/test/Ocelot.UnitTests/Logging/AspDotNetLoggerTests.cs @@ -0,0 +1,81 @@ +namespace Ocelot.UnitTests.Logging +{ + using Moq; + using Xunit; + using Ocelot.Logging; + using Microsoft.Extensions.Logging; + using Ocelot.Infrastructure.RequestData; + using System; + + public class AspDotNetLoggerTests + { + private readonly Mock> _coreLogger; + private readonly AspDotNetLogger _logger; + private Mock _repo; + private readonly string _b; + private readonly string _a; + private readonly Exception _ex; + + public AspDotNetLoggerTests() + { + _a = "tom"; + _b = "laura"; + _ex = new Exception("oh no"); + _coreLogger = new Mock>(); + _repo = new Mock(); + _logger = new AspDotNetLogger(_coreLogger.Object, _repo.Object); + } + + [Fact] + public void should_log_trace() + { + _logger.LogTrace($"a message from {_a} to {_b}"); + + ThenLevelIsLogged("requestId: no request id, previousRequestId: no previous request id, message: a message from tom to laura", LogLevel.Trace); + } + + [Fact] + public void should_log_info() + { + _logger.LogInformation($"a message from {_a} to {_b}"); + + ThenLevelIsLogged("requestId: no request id, previousRequestId: no previous request id, message: a message from tom to laura", LogLevel.Information); + } + + [Fact] + public void should_log_warning() + { + _logger.LogWarning($"a message from {_a} to {_b}"); + + ThenLevelIsLogged("requestId: no request id, previousRequestId: no previous request id, message: a message from tom to laura", LogLevel.Warning); + } + + [Fact] + public void should_log_error() + { + + _logger.LogError($"a message from {_a} to {_b}", _ex); + + ThenLevelIsLogged("requestId: no request id, previousRequestId: no previous request id, message: a message from tom to laura, exception: System.Exception: oh no", LogLevel.Error); + } + + [Fact] + public void should_log_critical() + { + _logger.LogCritical($"a message from {_a} to {_b}", _ex); + + ThenLevelIsLogged("requestId: no request id, previousRequestId: no previous request id, message: a message from tom to laura, exception: System.Exception: oh no", LogLevel.Critical); + } + + private void ThenLevelIsLogged(string expected, LogLevel expectedLogLevel) + { + _coreLogger.Verify( + x => x.Log( + expectedLogLevel, + It.IsAny(), + It.Is(o => o.ToString() == expected), + It.IsAny(), + It.IsAny>()), Times.Once); + } + } +} diff --git a/test/Ocelot.UnitTests/Logging/OcelotDiagnosticListenerTests.cs b/test/Ocelot.UnitTests/Logging/OcelotDiagnosticListenerTests.cs new file mode 100644 index 00000000..3703ddb0 --- /dev/null +++ b/test/Ocelot.UnitTests/Logging/OcelotDiagnosticListenerTests.cs @@ -0,0 +1,145 @@ +using Ocelot.Logging; +using Moq; +using TestStack.BDDfy; +using Butterfly.Client.Tracing; +using Ocelot.Requester; +using Xunit; +using Ocelot.Middleware; +using Microsoft.AspNetCore.Http; +using System; + +namespace Ocelot.UnitTests.Logging +{ + public class OcelotDiagnosticListenerTests + { + private readonly OcelotDiagnosticListener _listener; + private Mock _factory; + private readonly Mock _logger; + private IServiceTracer _tracer; + private DownstreamContext _downstreamContext; + private string _name; + private Exception _exception; + + public OcelotDiagnosticListenerTests() + { + _factory = new Mock(); + _logger = new Mock(); + _tracer = new FakeServiceTracer(); + _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); + _listener = new OcelotDiagnosticListener(_factory.Object, _tracer); + } + + [Fact] + public void should_trace_ocelot_middleware_started() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .When(_ => WhenOcelotMiddlewareStartedCalled()) + .Then(_ => ThenTheLogIs($"Ocelot.MiddlewareStarted: {_name}; {_downstreamContext.HttpContext.Request.Path}")) + .BDDfy(); + } + + [Fact] + public void should_trace_ocelot_middleware_finished() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .When(_ => WhenOcelotMiddlewareFinishedCalled()) + .Then(_ => ThenTheLogIs($"Ocelot.MiddlewareFinished: {_name}; {_downstreamContext.HttpContext.Request.Path}")) + .BDDfy(); + } + + [Fact] + public void should_trace_ocelot_middleware_exception() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .And(_ => GivenAException(new Exception("oh no"))) + .When(_ => WhenOcelotMiddlewareExceptionCalled()) + .Then(_ => ThenTheLogIs($"Ocelot.MiddlewareException: {_name}; {_exception.Message};")) + .BDDfy(); + } + + [Fact] + public void should_trace_middleware_started() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .When(_ => WhenMiddlewareStartedCalled()) + .Then(_ => ThenTheLogIs($"MiddlewareStarting: {_name}; {_downstreamContext.HttpContext.Request.Path}")) + .BDDfy(); + } + + [Fact] + public void should_trace_middleware_finished() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .When(_ => WhenMiddlewareFinishedCalled()) + .Then(_ => ThenTheLogIs($"MiddlewareFinished: {_name}; {_downstreamContext.HttpContext.Response.StatusCode}")) + .BDDfy(); + } + + [Fact] + public void should_trace_middleware_exception() + { + this.Given(_ => GivenAMiddlewareName()) + .And(_ => GivenAContext()) + .And(_ => GivenAException(new Exception("oh no"))) + .When(_ => WhenMiddlewareExceptionCalled()) + .Then(_ => ThenTheLogIs($"MiddlewareException: {_name}; {_exception.Message};")) + .BDDfy(); + } + + private void GivenAException(Exception exception) + { + _exception = exception; + } + + private void WhenOcelotMiddlewareStartedCalled() + { + _listener.OcelotMiddlewareStarted(_downstreamContext, _name); + } + + private void WhenOcelotMiddlewareFinishedCalled() + { + _listener.OcelotMiddlewareFinished(_downstreamContext, _name); + } + + private void WhenOcelotMiddlewareExceptionCalled() + { + _listener.OcelotMiddlewareException(_exception, _downstreamContext, _name); + } + + private void WhenMiddlewareStartedCalled() + { + _listener.OnMiddlewareStarting(_downstreamContext.HttpContext, _name); + } + + private void WhenMiddlewareFinishedCalled() + { + _listener.OnMiddlewareFinished(_downstreamContext.HttpContext, _name); + } + + private void WhenMiddlewareExceptionCalled() + { + _listener.OnMiddlewareException(_exception, _name); + } + + private void GivenAContext() + { + _downstreamContext = new DownstreamContext(new DefaultHttpContext()); + } + + private void GivenAMiddlewareName() + { + _name = "name"; + } + + private void ThenTheLogIs(string expected) + { + _logger.Verify( + x => x.LogTrace(expected)); + } + } +} diff --git a/test/Ocelot.UnitTests/Middleware/OcelotMiddlewareTests.cs b/test/Ocelot.UnitTests/Middleware/OcelotMiddlewareTests.cs new file mode 100644 index 00000000..fd4e018c --- /dev/null +++ b/test/Ocelot.UnitTests/Middleware/OcelotMiddlewareTests.cs @@ -0,0 +1,75 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Moq; +using Ocelot.Errors; +using Ocelot.Logging; +using Ocelot.Middleware; +using Ocelot.UnitTests.Responder; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Middleware +{ + public class OcelotMiddlewareTests + { + private Mock _logger; + private FakeMiddleware _middleware; + private List _errors; + + public OcelotMiddlewareTests() + { + _errors = new List(); + _logger = new Mock(); + _middleware = new FakeMiddleware(_logger.Object); + } + + [Fact] + public void should_log_error() + { + this.Given(x => GivenAnError(new AnyError())) + .When(x => WhenISetTheError()) + .Then(x => ThenTheErrorIsLogged(1)) + .BDDfy(); + } + + [Fact] + public void should_log_errors() + { + this.Given(x => GivenAnError(new AnyError())) + .And(x => GivenAnError(new AnyError())) + .When(x => WhenISetTheErrors()) + .Then(x => ThenTheErrorIsLogged(2)) + .BDDfy(); + } + + private void WhenISetTheErrors() + { + _middleware.SetPipelineError(new DownstreamContext(new DefaultHttpContext()), _errors); + } + + private void ThenTheErrorIsLogged(int times) + { + _logger.Verify(x => x.LogWarning("blahh"), Times.Exactly(times)); + } + + private void WhenISetTheError() + { + _middleware.SetPipelineError(new DownstreamContext(new DefaultHttpContext()), _errors[0]); + } + + private void GivenAnError(Error error) + { + _errors.Add(error); + } + + } + + public class FakeMiddleware : OcelotMiddleware + { + public FakeMiddleware(IOcelotLogger logger) + : base(logger) + { + } + } +} \ No newline at end of file diff --git a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj index 80084a4d..89af54d8 100644 --- a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj +++ b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj @@ -57,4 +57,8 @@ + + + + diff --git a/test/Ocelot.UnitTests/RequestId/ReRouteRequestIdMiddlewareTests.cs b/test/Ocelot.UnitTests/RequestId/ReRouteRequestIdMiddlewareTests.cs index 91c1e15c..44d9c405 100644 --- a/test/Ocelot.UnitTests/RequestId/ReRouteRequestIdMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/RequestId/ReRouteRequestIdMiddlewareTests.cs @@ -1,9 +1,6 @@ -using Ocelot.Middleware; - -namespace Ocelot.UnitTests.RequestId +namespace Ocelot.UnitTests.RequestId { using Microsoft.AspNetCore.Http; - using Microsoft.Extensions.Primitives; using Ocelot.Infrastructure.RequestData; using System; using System.Collections.Generic; @@ -21,6 +18,7 @@ namespace Ocelot.UnitTests.RequestId using TestStack.BDDfy; using Xunit; using Ocelot.Request.Middleware; + using Ocelot.Middleware; public class ReRouteRequestIdMiddlewareTests { @@ -142,6 +140,30 @@ namespace Ocelot.UnitTests.RequestId .BDDfy(); } + [Fact] + public void should_not_update_if_global_request_id_is_same_as_re_route_request_id() + { + var downstreamRoute = new DownstreamRoute(new List(), + new ReRouteBuilder() + .WithDownstreamReRoute(new DownstreamReRouteBuilder() + .WithDownstreamPathTemplate("any old string") + .WithRequestIdKey("LSRequestId") + .WithUpstreamHttpMethod(new List { "Get" }) + .Build()) + .WithUpstreamHttpMethod(new List { "Get" }) + .Build()); + + var requestId = "alreadyset"; + + this.Given(x => x.GivenTheDownStreamRouteIs(downstreamRoute)) + .And(x => GivenTheRequestIdWasSetGlobally()) + .And(x => x.GivenTheRequestIdIsAddedToTheRequest("LSRequestId", requestId)) + .When(x => x.WhenICallTheMiddleware()) + .Then(x => x.ThenTheTraceIdIs(requestId)) + .And(x => ThenTheRequestIdIsNotUpdated()) + .BDDfy(); + } + private void WhenICallTheMiddleware() { _middleware.Invoke(_downstreamContext).GetAwaiter().GetResult(); @@ -159,12 +181,17 @@ namespace Ocelot.UnitTests.RequestId private void ThenTheRequestIdIsSaved() { - _repo.Verify(x => x.Add("RequestId", _value), Times.Once); + _repo.Verify(x => x.Add("RequestId", _value), Times.Once); } private void ThenTheRequestIdIsUpdated() { - _repo.Verify(x => x.Update("RequestId", _value), Times.Once); + _repo.Verify(x => x.Update("RequestId", _value), Times.Once); + } + + private void ThenTheRequestIdIsNotUpdated() + { + _repo.Verify(x => x.Update("RequestId", _value), Times.Never); } private void GivenTheDownStreamRouteIs(DownstreamRoute downstreamRoute) @@ -182,15 +209,13 @@ namespace Ocelot.UnitTests.RequestId private void ThenTheTraceIdIsAnything() { - StringValues value; - _downstreamContext.HttpContext.Response.Headers.TryGetValue("LSRequestId", out value); + _downstreamContext.HttpContext.Response.Headers.TryGetValue("LSRequestId", out var value); value.First().ShouldNotBeNullOrEmpty(); } private void ThenTheTraceIdIs(string expected) { - StringValues value; - _downstreamContext.HttpContext.Response.Headers.TryGetValue("LSRequestId", out value); + _downstreamContext.HttpContext.Response.Headers.TryGetValue("LSRequestId", out var value); value.First().ShouldBe(expected); } } diff --git a/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs b/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs index 2e9ffcd6..54a77f4d 100644 --- a/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs @@ -51,7 +51,7 @@ namespace Ocelot.UnitTests.Responder public void should_return_any_errors() { this.Given(x => x.GivenTheHttpResponseMessageIs(new HttpResponseMessage())) - .And(x => x.GivenThereArePipelineErrors(new UnableToFindDownstreamRouteError())) + .And(x => x.GivenThereArePipelineErrors(new UnableToFindDownstreamRouteError("/path", "GET"))) .When(x => x.WhenICallTheMiddleware()) .Then(x => x.ThenThereAreNoErrors()) .BDDfy(); diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs index fc67ab8e..299da57f 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs @@ -142,12 +142,12 @@ namespace Ocelot.UnitTests.ServiceDiscovery private void ThenTheLoggerHasBeenCalledCorrectlyForInvalidAddress() { _logger.Verify( - x => x.LogError( + x => x.LogWarning( "Unable to use service Address: http://localhost and Port: 50881 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), Times.Once); _logger.Verify( - x => x.LogError( + x => x.LogWarning( "Unable to use service Address: http://localhost and Port: 50888 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), Times.Once); } @@ -155,12 +155,12 @@ namespace Ocelot.UnitTests.ServiceDiscovery private void ThenTheLoggerHasBeenCalledCorrectlyForInvalidPorts() { _logger.Verify( - x => x.LogError( + x => x.LogWarning( "Unable to use service Address: localhost and Port: -1 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), Times.Once); _logger.Verify( - x => x.LogError( + x => x.LogWarning( "Unable to use service Address: localhost and Port: 0 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), Times.Once); }