Improve log and errors (#2747)

* Rename SerilogHelper to SerilogWebRequestHelper * Move LoggedUser property to every log (instead of just WebRequestLogging) * Add ProjectId to logs * Improve naming on error page
joinrpg · Aug 14, 2024 · 3c8db64 · 3c8db64
1 parent 5a34c3b
commit 3c8db64
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 33 deletions.
diff --git a/src/JoinRpg.Portal/Controllers/ErrorPageController.cs b/src/JoinRpg.Portal/Controllers/ErrorPageController.cs
@@ -49,8 +49,8 @@ public IActionResult Error(int statusCode)
         return View(
             new ErrorViewModel
             {
-                RequestId = Activity.Current?.Id ?? "",
-                AspNetTrace = HttpContext.TraceIdentifier,
+                ActivityId = Activity.Current?.Id,
+                RequestId = HttpContext.TraceIdentifier,
                 Path = feature?.RawTarget ?? "NO PATH",
             }
             );

diff --git a/src/JoinRpg.Portal/Infrastructure/DiscoverFilters/DiscoverProjectMiddleware.cs b/src/JoinRpg.Portal/Infrastructure/DiscoverFilters/DiscoverProjectMiddleware.cs
@@ -1,3 +1,5 @@
+using Serilog.Context;
+
 namespace JoinRpg.Portal.Infrastructure.DiscoverFilters;
 
 /// <summary>
@@ -12,15 +14,12 @@ public class DiscoverProjectMiddleware
     /// <inheritedoc />
     public async Task InvokeAsync(HttpContext context)
     {
-        var httpContextItems = context.Items;
+        HttpRequest request = context.Request;
 
-        if (context.Request.Path.TryExtractFromPath() is int projectIdFromPath)
-        {
-            httpContextItems[Constants.ProjectIdName] = projectIdFromPath;
-        }
-        else if (context.Request.Query.TryExtractFromQuery() is int projectIdFromQuery)
+        if ((request.Path.TryExtractFromPath() ?? request.Query.TryExtractFromQuery()) is int projectId)
         {
-            httpContextItems[Constants.ProjectIdName] = projectIdFromQuery;
+            context.Items[Constants.ProjectIdName] = projectId;
+            _ = LogContext.PushProperty(Constants.ProjectIdName, projectId);
         }
 
         await nextDelegate(context);

diff --git a/src/JoinRpg.Portal/Infrastructure/Logging/LoggedUserEnricher.cs b/src/JoinRpg.Portal/Infrastructure/Logging/LoggedUserEnricher.cs
@@ -0,0 +1,16 @@
+using System.Security.Claims;
+using Serilog.Core;
+using Serilog.Events;
+
+namespace JoinRpg.Portal.Infrastructure.Logging;
+
+internal class LoggedUserEnricher(IHttpContextAccessor httpContextAccessor) : ILogEventEnricher
+{
+    public LoggedUserEnricher() : this(new HttpContextAccessor()) { }
+    public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
+    {
+        logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty(
+                "LoggedUser", httpContextAccessor.HttpContext?.User.FindFirst(ClaimTypes.Email)?.Value));
+    }
+}
+
diff --git a/src/JoinRpg.Portal/Infrastructure/Logging/SerilogExtensions.cs b/src/JoinRpg.Portal/Infrastructure/Logging/SerilogExtensions.cs
@@ -22,6 +22,7 @@ public static void ConfigureLogger(this LoggerConfiguration loggerConfiguration,
             .Enrich.FromLogContext()
             .Enrich.WithMachineName()
             .Enrich.With<YcLevelEnricher>()
+            .Enrich.With<LoggedUserEnricher>()
             .Enrich.WithProperty("AppName", "JoinRpg.Portal");
 
         foreach (var (@namespace, logLevel) in serilogOptions.LogLevel)

diff --git a/...l/Infrastructure/Logging/SerilogHelper.cs → ...ucture/Logging/SerilogWebRequestHelper.cs b/...l/Infrastructure/Logging/SerilogHelper.cs → ...ucture/Logging/SerilogWebRequestHelper.cs
@@ -1,10 +1,9 @@
-using System.Security.Claims;
 using Serilog;
 using Serilog.Events;
 
 namespace JoinRpg.Portal.Infrastructure.Logging;
 
-public static class SerilogHelper
+public static class SerilogWebRequestHelper
 {
     public static void EnrichFromRequest(IDiagnosticContext diagnosticContext, HttpContext httpContext)
     {
@@ -30,15 +29,6 @@ public static void EnrichFromRequest(IDiagnosticContext diagnosticContext, HttpC
         {
             diagnosticContext.Set("EndpointName", endpoint.DisplayName);
         }
-
-        if (httpContext.User.Identity?.IsAuthenticated == true)
-        {
-            diagnosticContext.Set("LoggedUser", httpContext.User.FindFirst(ClaimTypes.Email)!.Value);
-        }
-        else
-        {
-            diagnosticContext.Set("LoggedUser", "null");
-        }
     }
     private static bool IsHealthCheckEndpoint(HttpContext ctx)
     {

diff --git a/src/JoinRpg.Portal/Models/ErrorViewModel.cs b/src/JoinRpg.Portal/Models/ErrorViewModel.cs
@@ -2,11 +2,9 @@ namespace JoinRpg.Portal.Models;
 
 public class ErrorViewModel
 {
-    public required string RequestId { get; set; }
-
-    public bool ShowRequestId => !string.IsNullOrEmpty(RequestId);
+    public required string? ActivityId { get; set; }
 
     public required string Path { get; set; }
 
-    public required string AspNetTrace { get; set; }
+    public required string RequestId { get; set; }
 }
diff --git a/src/JoinRpg.Portal/Program.cs b/src/JoinRpg.Portal/Program.cs
@@ -34,7 +34,7 @@ public static IHostBuilder CreateHostBuilder(string[] args) =>
             .UseSerilog((context, _, configuration) =>
             {
                 var loggerOptions = context.Configuration.GetSection("Logging").Get<SerilogOptions>();
-                configuration.ConfigureLogger(loggerOptions);
+                configuration.ConfigureLogger(loggerOptions!);
             })
             .ConfigureWebHostDefaults(webBuilder => webBuilder.UseStartup<Startup>());
 }
diff --git a/src/JoinRpg.Portal/Startup.cs b/src/JoinRpg.Portal/Startup.cs
@@ -180,8 +180,8 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
 
         _ = app.UseSerilogRequestLogging(opts =>
         {
-            opts.EnrichDiagnosticContext = SerilogHelper.EnrichFromRequest;
-            opts.GetLevel = SerilogHelper.ExcludeHealthChecks;
+            opts.EnrichDiagnosticContext = SerilogWebRequestHelper.EnrichFromRequest;
+            opts.GetLevel = SerilogWebRequestHelper.ExcludeHealthChecks;
         });
 
         _ = app

diff --git a/src/JoinRpg.Portal/Views/Shared/Error.cshtml b/src/JoinRpg.Portal/Views/Shared/Error.cshtml
@@ -10,15 +10,12 @@
     Обратитесь в техподдержку!
 </p>
 
-@if (Model.ShowRequestId)
+@if (Model.ActivityId is not null)
 {
-    <p> RequestId: @Model.RequestId </p>
+    <p> Activity: @Model.ActivityId </p>
 }
 
-@if (Model.AspNetTrace is not null)
-{
-        <p>TraceIdentifier: @Model.AspNetTrace</p>
-}
+<p>RequestId: @Model.RequestId</p>
 
 @if (!string.IsNullOrWhiteSpace(Model.Path))
 {