Skip to content

Commit

Permalink
adding back the logger methods with string as parameter, avoiding cal…
Browse files Browse the repository at this point in the history
…ling the factory when plain string are used.
  • Loading branch information
ggnaegi committed Oct 19, 2023
1 parent 847dac7 commit d7a8397
Show file tree
Hide file tree
Showing 15 changed files with 170 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
}
catch (BrokenCircuitException ex)
{
_logger.LogError(() => "Reached to allowed number of exceptions. Circuit is open", ex);
_logger.LogError("Reached to allowed number of exceptions. Circuit is open", ex);
throw;
}
catch (HttpRequestException ex)
Expand Down
10 changes: 5 additions & 5 deletions src/Ocelot/Authorization/Middleware/AuthorizationMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,25 @@ public async Task Invoke(HttpContext httpContext)

if (!IsOptionsHttpMethod(httpContext) && IsAuthenticatedRoute(downstreamRoute))
{
Logger.LogInformation(() =>"route is authenticated scopes must be checked");
Logger.LogInformation("route is authenticated scopes must be checked");

var authorized = _scopesAuthorizer.Authorize(httpContext.User, downstreamRoute.AuthenticationOptions.AllowedScopes);

if (authorized.IsError)
{
Logger.LogWarning(() => "error authorizing user scopes");
Logger.LogWarning("error authorizing user scopes");

httpContext.Items.UpsertErrors(authorized.Errors);
return;
}

if (IsAuthorized(authorized))
{
Logger.LogInformation(() => "user scopes is authorized calling next authorization checks");
Logger.LogInformation("user scopes is authorized calling next authorization checks");
}
else
{
Logger.LogWarning(() =>"user scopes is not authorized setting pipeline error");
Logger.LogWarning("user scopes is not authorized setting pipeline error");

httpContext.Items.SetError(new UnauthorizedError(
$"{httpContext.User.Identity.Name} unable to access {downstreamRoute.UpstreamPathTemplate.OriginalValue}"));
Expand All @@ -56,7 +56,7 @@ public async Task Invoke(HttpContext httpContext)

if (!IsOptionsHttpMethod(httpContext) && IsAuthorizedRoute(downstreamRoute))
{
Logger.LogInformation(() => "route is authorized");
Logger.LogInformation("route is authorized");

var authorized = _claimsAuthorizer.Authorize(httpContext.User, downstreamRoute.RouteClaimsRequirement, httpContext.Items.TemplatePlaceholderNameAndValues());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public Task StopAsync(CancellationToken cancellationToken)

private async Task Poll()
{
_logger.LogInformation(() => "Started polling");
_logger.LogInformation("Started polling");

var fileConfig = await _repo.Get();

Expand All @@ -86,7 +86,7 @@ private async Task Poll()
_previousAsJson = asJson;
}

_logger.LogInformation(() => "Finished polling");
_logger.LogInformation("Finished polling");
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public async Task Invoke(HttpContext httpContext)

if (response.IsError)
{
Logger.LogWarning(() => "there was an error setting queries on context, setting pipeline error");
Logger.LogWarning("there was an error setting queries on context, setting pipeline error");

httpContext.Items.UpsertErrors(response.Errors);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ public async Task Invoke(HttpContext httpContext)
return;
}

Logger.LogDebug(() =>
{
var downstreamPathTemplates = string.Join(", ", response.Data.Route.DownstreamRoute.Select(r => r.DownstreamPathTemplate.Value));
return $"downstream templates are {downstreamPathTemplates}";
});
Logger.LogDebug(() => $"downstream templates are {string.Join(", ", response.Data.Route.DownstreamRoute.Select(r => r.DownstreamPathTemplate.Value))}");

// why set both of these on HttpContext
httpContext.Items.UpsertTemplatePlaceholderNameAndValues(response.Data.TemplatePlaceholderNameAndValues);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public async Task Invoke(HttpContext httpContext)

if (response.IsError)
{
Logger.LogDebug(() => "IDownstreamPathPlaceholderReplacer returned an error, setting pipeline error");
Logger.LogDebug("IDownstreamPathPlaceholderReplacer returned an error, setting pipeline error");

httpContext.Items.UpsertErrors(response.Errors);
return;
Expand Down
8 changes: 4 additions & 4 deletions src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,27 @@ public async Task Invoke(HttpContext httpContext)

TrySetGlobalRequestId(httpContext, internalConfiguration);

Logger.LogDebug(() =>"ocelot pipeline started");
Logger.LogDebug("ocelot pipeline started");

await _next.Invoke(httpContext);
}
catch (OperationCanceledException) when (httpContext.RequestAborted.IsCancellationRequested)
{
Logger.LogDebug(() =>"operation canceled");
Logger.LogDebug("operation canceled");
if (!httpContext.Response.HasStarted)
{
httpContext.Response.StatusCode = 499;
}
}
catch (Exception e)
{
Logger.LogDebug(() =>"error calling middleware");
Logger.LogDebug("error calling middleware");
Logger.LogError(() => CreateMessage(httpContext, e), e);

SetInternalServerErrorOnResponse(httpContext);
}

Logger.LogDebug(() =>"ocelot pipeline finished");
Logger.LogDebug("ocelot pipeline finished");
}

private void TrySetGlobalRequestId(HttpContext httpContext, IInternalConfiguration configuration)
Expand Down
4 changes: 2 additions & 2 deletions src/Ocelot/Headers/Middleware/ClaimsToHeadersMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ public async Task Invoke(HttpContext httpContext)

if (response.IsError)
{
Logger.LogWarning(() => "Error setting headers on context, setting pipeline error");
Logger.LogWarning( "Error setting headers on context, setting pipeline error");

httpContext.Items.UpsertErrors(response.Errors);
return;
}

Logger.LogInformation(() => "headers have been set on context");
Logger.LogInformation("headers have been set on context");
}

await _next.Invoke(httpContext);
Expand Down
6 changes: 3 additions & 3 deletions src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ public async Task Invoke(HttpContext httpContext)

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");
httpContext.Items.UpsertErrors(loadBalancer.Errors);
return;
}

var hostAndPort = await loadBalancer.Data.Lease(httpContext);
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");
httpContext.Items.UpsertErrors(hostAndPort.Errors);
return;
}
Expand All @@ -63,7 +63,7 @@ public async Task Invoke(HttpContext httpContext)
}
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
Expand Down
178 changes: 104 additions & 74 deletions src/Ocelot/Logging/AspDotNetLogger.cs
Original file line number Diff line number Diff line change
@@ -1,78 +1,108 @@
using Microsoft.Extensions.Logging;
using Ocelot.Infrastructure.RequestData;

namespace Ocelot.Logging
{
public class AspDotNetLogger : IOcelotLogger
{
private readonly ILogger _logger;
private readonly IRequestScopedDataRepository _scopedDataRepository;
private readonly Func<string, Exception, string> _func;

public AspDotNetLogger(ILogger logger, IRequestScopedDataRepository scopedDataRepository)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_scopedDataRepository = scopedDataRepository ?? throw new ArgumentNullException(nameof(scopedDataRepository));
_func = (state, exception) => exception == null ? state : $"{state}, exception: {exception}";
}

public void LogTrace(Func<string> messageFactory)
{
WriteLog(LogLevel.Trace, messageFactory);
}

public void LogDebug(Func<string> messageFactory)

namespace Ocelot.Logging;

public class AspDotNetLogger : IOcelotLogger
{
private readonly ILogger _logger;
private readonly IRequestScopedDataRepository _scopedDataRepository;
private readonly Func<string, Exception, string> _func;

public AspDotNetLogger(ILogger logger, IRequestScopedDataRepository scopedDataRepository)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_scopedDataRepository = scopedDataRepository;
_func = (state, exception) => exception == null ? state : $"{state}, exception: {exception}";
}

public void LogTrace(string message)
{
LogTrace(() => message);
}

public void LogTrace(Func<string> messageFactory)
{
WriteLog(LogLevel.Trace, messageFactory);
}

public void LogDebug(string message)
{
LogDebug(() => message);
}

public void LogDebug(Func<string> messageFactory)
{
WriteLog(LogLevel.Debug, messageFactory);
}

public void LogInformation(string message)
{
LogInformation(() => message);
}

public void LogInformation(Func<string> messageFactory)
{
WriteLog(LogLevel.Information, messageFactory);
}

public void LogWarning(string message)
{
LogWarning(() => message);
}

public void LogWarning(Func<string> messageFactory)
{
WriteLog(LogLevel.Warning, messageFactory);
}

public void LogError(string message, Exception exception)
{
LogError(() => message, exception);
}

public void LogError(Func<string> messageFactory, Exception exception)
{
WriteLog(LogLevel.Error, messageFactory, exception);
}

public void LogCritical(string message, Exception exception)
{
LogCritical(() => message, exception);
}

public void LogCritical(Func<string> messageFactory, Exception exception)
{
WriteLog(LogLevel.Critical, messageFactory, exception);
}

private string GetOcelotRequestId()
{
var requestId = _scopedDataRepository.Get<string>("RequestId");

return requestId == null || requestId.IsError ? "no request id" : requestId.Data;
}

private string GetOcelotPreviousRequestId()
{
var requestId = _scopedDataRepository.Get<string>("PreviousRequestId");

return requestId == null || requestId.IsError ? "no previous request id" : requestId.Data;
}

public void WriteLog(LogLevel logLevel, Func<string> messageFactory, Exception exception = null)
{
if (!_logger.IsEnabled(logLevel))
{
WriteLog(LogLevel.Debug, messageFactory);
}

public void LogInformation(Func<string> messageFactory)
{
WriteLog(LogLevel.Information, messageFactory);
}

public void LogWarning(Func<string> messageFactory)
{
WriteLog(LogLevel.Warning, messageFactory);
}

public void LogError(Func<string> messageFactory, Exception exception)
{
WriteLog(LogLevel.Error, messageFactory, exception);
}

public void LogCritical(Func<string> messageFactory, Exception exception)
{
WriteLog(LogLevel.Critical, messageFactory, exception);
}

private string GetOcelotRequestId()
{
var requestId = _scopedDataRepository.Get<string>("RequestId");

return requestId == null || requestId.IsError ? "no request id" : requestId.Data;
}

private string GetOcelotPreviousRequestId()
{
var requestId = _scopedDataRepository.Get<string>("PreviousRequestId");

return requestId == null || requestId.IsError ? "no previous request id" : requestId.Data;
}

private void WriteLog(LogLevel logLevel, Func<string> messageFactory, Exception exception = null)
{
if (!_logger.IsEnabled(logLevel))
{
return;
}

var requestId = GetOcelotRequestId();
var previousRequestId = GetOcelotPreviousRequestId();

var state = $"requestId: {requestId}, previousRequestId: {previousRequestId}, message: {messageFactory.Invoke()}";

_logger.Log(logLevel, default, state, exception, _func);
}
}
return;
}

var requestId = GetOcelotRequestId();
var previousRequestId = GetOcelotPreviousRequestId();

var state =
$"requestId: {requestId}, previousRequestId: {previousRequestId}, message: {messageFactory.Invoke()}";

_logger.Log(logLevel, default, state, exception, _func);
}
}
33 changes: 18 additions & 15 deletions src/Ocelot/Logging/IOcelotLogger.cs
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
using System.Diagnostics;
namespace Ocelot.Logging;

namespace Ocelot.Logging
/// <summary>
/// Thin wrapper around the DotNet core logging framework, used to allow the scopedDataRepository to be injected giving access to the Ocelot RequestId.
/// </summary>
public interface IOcelotLogger
{
/// <summary>
/// Thin wrapper around the DotNet core logging framework, used to allow the scopedDataRepository to be injected giving access to the Ocelot RequestId.
/// </summary>
public interface IOcelotLogger
{
void LogTrace(Func<string> messageFactory);

void LogDebug(Func<string> messageFactory);
void LogTrace(string message);
void LogTrace(Func<string> messageFactory);

void LogInformation(Func<string> messageFactory);
void LogDebug(string message);
void LogDebug(Func<string> messageFactory);

void LogWarning(Func<string> messageFactory);
void LogInformation(string message);
void LogInformation(Func<string> messageFactory);

void LogError(Func<string> messageFactory, Exception exception);
void LogWarning(string message);
void LogWarning(Func<string> messageFactory);

void LogCritical(Func<string> messageFactory, Exception exception);
}
void LogError(string message, Exception exception);
void LogError(Func<string> messageFactory, Exception exception);

void LogCritical(string message, Exception exception);
void LogCritical(Func<string> messageFactory, Exception exception);
}
Loading

0 comments on commit d7a8397

Please sign in to comment.