From cb2ac8e1f182585c78eec430e781be4a493a3f80 Mon Sep 17 00:00:00 2001 From: KonstantinRyazantsev Date: Mon, 25 Jun 2018 16:31:52 +0300 Subject: [PATCH 1/2] Logging system redesign. Backward compatibility restored --- .../ClientErrorHandlerMiddleware.cs | 25 ++++++++--- .../GlobalErrorHandlerMiddleware.cs | 24 +++++++--- .../MiddlewareApplicationBuilderExtensions.cs | 44 ++++++++++++++++++- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/Lykke.Common.ApiLibrary/Middleware/ClientErrorHandlerMiddleware.cs b/src/Lykke.Common.ApiLibrary/Middleware/ClientErrorHandlerMiddleware.cs index 3830154..eb14d2e 100644 --- a/src/Lykke.Common.ApiLibrary/Middleware/ClientErrorHandlerMiddleware.cs +++ b/src/Lykke.Common.ApiLibrary/Middleware/ClientErrorHandlerMiddleware.cs @@ -17,6 +17,18 @@ public class ClientErrorHandlerMiddleware private readonly ILog _log; private readonly RequestDelegate _next; + [Obsolete] + public ClientErrorHandlerMiddleware(RequestDelegate next, ILog log, string componentName) + { + if (log == null) + { + throw new ArgumentNullException(nameof(log)); + } + + _log = log.CreateComponentScope(componentName); + _next = next; + } + /// /// Logs url, request body and response status for responses with status code = 4xx /// @@ -48,12 +60,13 @@ private async Task LogError(HttpContext context) var urlWithoutQuery = RequestUtils.GetUrlWithoutQuery(url) ?? "?"; var body = await RequestUtils.GetRequestPartialBodyAsync(context); - _log.Warning(urlWithoutQuery, message: null, context: new - { - url = url, - statusCode = context.Response.StatusCode, - body = body - }); + _log.WriteWarning(urlWithoutQuery, new + { + url = url, + statusCode = context.Response.StatusCode, + body = body + }, + ""); } } } \ No newline at end of file diff --git a/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs b/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs index cc17da7..ba3f84b 100644 --- a/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs +++ b/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs @@ -19,6 +19,19 @@ public class GlobalErrorHandlerMiddleware private readonly CreateErrorResponse _createErrorResponse; private readonly RequestDelegate _next; + [Obsolete] + public GlobalErrorHandlerMiddleware(RequestDelegate next, ILog log, string componentName, CreateErrorResponse createErrorResponse) + { + if (log == null) + { + throw new ArgumentNullException(nameof(log)); + } + + _log = log.CreateComponentScope(componentName); + _createErrorResponse = createErrorResponse ?? throw new ArgumentNullException(nameof(createErrorResponse)); + _next = next; _next = next; + } + /// /// Middleware that handles all unhandled exceptions and use delegate to generate error response /// @@ -53,11 +66,12 @@ private async Task LogError(HttpContext context, Exception ex) var urlWithoutQuery = RequestUtils.GetUrlWithoutQuery(url) ?? "?"; var body = await RequestUtils.GetRequestPartialBodyAsync(context); - _log.Error(urlWithoutQuery, ex, context: new - { - url = url, - body = body - }); + _log.WriteError(urlWithoutQuery, new + { + url = url, + body = body + }, + ex); } private async Task CreateErrorResponse(HttpContext ctx, Exception ex) diff --git a/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs b/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs index e060800..31337e1 100644 --- a/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs +++ b/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs @@ -1,14 +1,52 @@ using System; +using Common.Log; using JetBrains.Annotations; +using Lykke.Common.Log; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http.Internal; using Microsoft.AspNetCore.HttpOverrides; +using Microsoft.Extensions.DependencyInjection; namespace Lykke.Common.ApiLibrary.Middleware { [PublicAPI] public static class MiddlewareApplicationBuilderExtensions { + /// + /// Configure application to use standart Lykke middleware + /// + /// Application builder + /// Component name for logs + /// Create global error response delegate + /// log 4xx errors + [Obsolete] + public static void UseLykkeMiddleware( + this IApplicationBuilder app, + string componentName, + CreateErrorResponse createGlobalErrorResponse, + bool logClientErrors = false) + { + app.Use(async (context, next) => + { + // enable ability to seek on request stream within any host, + // but not only Kestrel, for any subsequent middleware + context.Request.EnableRewind(); + await next(); + }); + + app.UseMiddleware( + app.ApplicationServices.GetRequiredService(), + componentName, + createGlobalErrorResponse); + + if (logClientErrors) + { + app.UseMiddleware( + app.ApplicationServices.GetRequiredService(), + componentName); + } + } + /// /// Configure application to use standart Lykke middleware /// @@ -37,11 +75,13 @@ public static void UseLykkeMiddleware( await next(); }); - app.UseMiddleware(createGlobalErrorResponse); + app.UseMiddleware( + app.ApplicationServices.GetRequiredService(), + createGlobalErrorResponse); if (logClientErrors) { - app.UseMiddleware(); + app.UseMiddleware(app.ApplicationServices.GetRequiredService()); } } From 0a8efdbb201da930d688eec5beade4024f9b52ba Mon Sep 17 00:00:00 2001 From: KonstantinRyazantsev Date: Mon, 25 Jun 2018 16:45:12 +0300 Subject: [PATCH 2/2] Loggig system redesign. Review remarks have been fixed --- .../Middleware/GlobalErrorHandlerMiddleware.cs | 4 ++-- .../MiddlewareApplicationBuilderExtensions.cs | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs b/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs index ba3f84b..0c91ad1 100644 --- a/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs +++ b/src/Lykke.Common.ApiLibrary/Middleware/GlobalErrorHandlerMiddleware.cs @@ -28,8 +28,8 @@ public GlobalErrorHandlerMiddleware(RequestDelegate next, ILog log, string compo } _log = log.CreateComponentScope(componentName); - _createErrorResponse = createErrorResponse ?? throw new ArgumentNullException(nameof(createErrorResponse)); - _next = next; _next = next; + _createErrorResponse = createErrorResponse ?? throw new ArgumentNullException(nameof(createErrorResponse)); + _next = next; } /// diff --git a/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs b/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs index 31337e1..f65bbdb 100644 --- a/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs +++ b/src/Lykke.Common.ApiLibrary/Middleware/MiddlewareApplicationBuilderExtensions.cs @@ -34,15 +34,17 @@ public static void UseLykkeMiddleware( await next(); }); + var log = app.ApplicationServices.GetRequiredService(); + app.UseMiddleware( - app.ApplicationServices.GetRequiredService(), + log, componentName, createGlobalErrorResponse); if (logClientErrors) { app.UseMiddleware( - app.ApplicationServices.GetRequiredService(), + log, componentName); } } @@ -75,13 +77,15 @@ public static void UseLykkeMiddleware( await next(); }); + var logFactory = app.ApplicationServices.GetRequiredService(); + app.UseMiddleware( - app.ApplicationServices.GetRequiredService(), + logFactory, createGlobalErrorResponse); if (logClientErrors) { - app.UseMiddleware(app.ApplicationServices.GetRequiredService()); + app.UseMiddleware(logFactory); } }