From 18b285f088df1508fb7b9686e78d750b962e7c3a Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 22 Apr 2020 15:23:51 +1000 Subject: [PATCH] Splits profiler so it doesn't have to take a dependency since we don't want to pre-resolve. Adds enrichers to the container. --- src/Umbraco.Core/Logging/IProfiler.cs | 8 +-- src/Umbraco.Core/Logging/IProfilerHtml.cs | 15 ++++++ src/Umbraco.Core/Logging/LogProfiler.cs | 6 --- src/Umbraco.Core/Logging/VoidProfiler.cs | 5 -- .../ThreadAbortExceptionEnricher.cs | 6 +-- .../Logging/Serilog/SerilogComposer.cs | 21 ++++++++ .../TestHelpers/Stubs/TestProfiler.cs | 5 -- .../UmbracoApplicationBuilderExtensions.cs | 6 +++ .../UmbracoCoreServiceCollectionExtensions.cs | 9 ++-- .../UmbracoRequestLoggingMiddleware.cs | 21 +++++--- .../Runtime/AspNetCoreComposer.cs | 7 +++ .../Runtime/Profiler/WebProfiler.cs | 44 +--------------- .../Runtime/Profiler/WebProfilerComposer.cs | 9 +++- .../Runtime/Profiler/WebProfilerHtml.cs | 51 +++++++++++++++++++ src/Umbraco.Web.UI.NetCore/Startup.cs | 3 +- src/Umbraco.Web/Composing/Current.cs | 2 + src/Umbraco.Web/HtmlHelperRenderExtensions.cs | 2 +- src/Umbraco.Web/Logging/WebProfiler.cs | 2 +- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 6 ++- 19 files changed, 140 insertions(+), 88 deletions(-) create mode 100644 src/Umbraco.Core/Logging/IProfilerHtml.cs rename src/Umbraco.Infrastructure/Logging/Serilog/{ => Enrichers}/ThreadAbortExceptionEnricher.cs (98%) create mode 100644 src/Umbraco.Infrastructure/Logging/Serilog/SerilogComposer.cs create mode 100644 src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerHtml.cs diff --git a/src/Umbraco.Core/Logging/IProfiler.cs b/src/Umbraco.Core/Logging/IProfiler.cs index 1327651197..d855612c95 100644 --- a/src/Umbraco.Core/Logging/IProfiler.cs +++ b/src/Umbraco.Core/Logging/IProfiler.cs @@ -2,18 +2,12 @@ namespace Umbraco.Core.Logging { + /// /// Defines the profiling service. /// public interface IProfiler { - /// - /// Renders the profiling results. - /// - /// The profiling results. - /// Generally used for HTML rendering. - string Render(); - /// /// Gets an that will time the code between its creation and disposal. /// diff --git a/src/Umbraco.Core/Logging/IProfilerHtml.cs b/src/Umbraco.Core/Logging/IProfilerHtml.cs new file mode 100644 index 0000000000..4f9ee62e0b --- /dev/null +++ b/src/Umbraco.Core/Logging/IProfilerHtml.cs @@ -0,0 +1,15 @@ +namespace Umbraco.Core.Logging +{ + /// + /// Used to render a profiler in a web page + /// + public interface IProfilerHtml + { + /// + /// Renders the profiling results. + /// + /// The profiling results. + /// Generally used for HTML rendering. + string Render(); + } +} diff --git a/src/Umbraco.Core/Logging/LogProfiler.cs b/src/Umbraco.Core/Logging/LogProfiler.cs index 294f92dad3..a1d2a2e61f 100644 --- a/src/Umbraco.Core/Logging/LogProfiler.cs +++ b/src/Umbraco.Core/Logging/LogProfiler.cs @@ -15,12 +15,6 @@ namespace Umbraco.Core.Logging _logger = logger; } - /// - public string Render() - { - return string.Empty; - } - /// public IDisposable Step(string name) { diff --git a/src/Umbraco.Core/Logging/VoidProfiler.cs b/src/Umbraco.Core/Logging/VoidProfiler.cs index 51bec521a3..d771fd7630 100644 --- a/src/Umbraco.Core/Logging/VoidProfiler.cs +++ b/src/Umbraco.Core/Logging/VoidProfiler.cs @@ -6,11 +6,6 @@ namespace Umbraco.Core.Logging { private readonly VoidDisposable _disposable = new VoidDisposable(); - public string Render() - { - return string.Empty; - } - public IDisposable Step(string name) { return _disposable; diff --git a/src/Umbraco.Infrastructure/Logging/Serilog/ThreadAbortExceptionEnricher.cs b/src/Umbraco.Infrastructure/Logging/Serilog/Enrichers/ThreadAbortExceptionEnricher.cs similarity index 98% rename from src/Umbraco.Infrastructure/Logging/Serilog/ThreadAbortExceptionEnricher.cs rename to src/Umbraco.Infrastructure/Logging/Serilog/Enrichers/ThreadAbortExceptionEnricher.cs index 2a7d35b636..1f495d3a50 100644 --- a/src/Umbraco.Infrastructure/Logging/Serilog/ThreadAbortExceptionEnricher.cs +++ b/src/Umbraco.Infrastructure/Logging/Serilog/Enrichers/ThreadAbortExceptionEnricher.cs @@ -7,7 +7,7 @@ using Umbraco.Core.Configuration; using Umbraco.Core.Diagnostics; using Umbraco.Core.Hosting; -namespace Umbraco.Core.Logging.Serilog +namespace Umbraco.Infrastructure.Logging.Serilog.Enrichers { /// /// Enriches the log if there are ThreadAbort exceptions and will automatically create a minidump if it can @@ -54,7 +54,6 @@ namespace Umbraco.Core.Logging.Serilog logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty("ThreadAbortExceptionInfo", message)); } else - { try { var dumped = MiniDump.Dump(_marchal, _hostingEnvironment, withException: true); @@ -68,7 +67,6 @@ namespace Umbraco.Core.Logging.Serilog message = "Failed to create a minidump. " + ex; logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty("ThreadAbortExceptionInfo", message)); } - } } private static bool IsTimeoutThreadAbortException(Exception exception) @@ -93,6 +91,6 @@ namespace Umbraco.Core.Logging.Serilog return stacktrace.Contains("System.Threading.Monitor.ReliableEnter"); } - + } } diff --git a/src/Umbraco.Infrastructure/Logging/Serilog/SerilogComposer.cs b/src/Umbraco.Infrastructure/Logging/Serilog/SerilogComposer.cs new file mode 100644 index 0000000000..18b417d428 --- /dev/null +++ b/src/Umbraco.Infrastructure/Logging/Serilog/SerilogComposer.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Umbraco.Core; +using Umbraco.Core.Composing; +using Umbraco.Core.Logging.Serilog.Enrichers; +using Umbraco.Infrastructure.Logging.Serilog.Enrichers; + +namespace Umbraco.Infrastructure.Logging.Serilog +{ + public class SerilogComposer : ICoreComposer + { + public void Compose(Composition composition) + { + composition.RegisterUnique(); + composition.RegisterUnique(); + composition.RegisterUnique(); + composition.RegisterUnique(); + } + } +} diff --git a/src/Umbraco.Tests/TestHelpers/Stubs/TestProfiler.cs b/src/Umbraco.Tests/TestHelpers/Stubs/TestProfiler.cs index 39cac6e24f..ea0f9cc44f 100644 --- a/src/Umbraco.Tests/TestHelpers/Stubs/TestProfiler.cs +++ b/src/Umbraco.Tests/TestHelpers/Stubs/TestProfiler.cs @@ -19,11 +19,6 @@ namespace Umbraco.Tests.TestHelpers.Stubs private static bool _enabled; - public string Render() - { - return string.Empty; - } - public IDisposable Step(string name) { return _enabled ? MiniProfiler.Current.Step(name) : null; diff --git a/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoApplicationBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoApplicationBuilderExtensions.cs index 88b9b4ca9e..a27113e881 100644 --- a/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoApplicationBuilderExtensions.cs @@ -4,7 +4,9 @@ using Microsoft.Extensions.DependencyInjection; using Serilog.Context; using Smidge; using Umbraco.Core; +using Umbraco.Core.Configuration; using Umbraco.Core.Hosting; +using Umbraco.Infrastructure.Logging.Serilog.Enrichers; using Umbraco.Web.Common.Middleware; namespace Umbraco.Web.BackOffice.AspNetCore @@ -33,6 +35,10 @@ namespace Umbraco.Web.BackOffice.AspNetCore var runtimeShutdown = new CoreRuntimeShutdown(runtime, hostLifetime); hostLifetime.RegisterObject(runtimeShutdown); + // Register our global threadabort enricher for logging + var threadAbortEnricher = app.ApplicationServices.GetRequiredService(); + LogContext.Push(threadAbortEnricher); // NOTE: We are not in a using clause because we are not removing it, it is on the global context + // Start the runtime! runtime.Start(); diff --git a/src/Umbraco.Web.Common/Extensions/UmbracoCoreServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/Extensions/UmbracoCoreServiceCollectionExtensions.cs index 80482852f3..2439243f30 100644 --- a/src/Umbraco.Web.Common/Extensions/UmbracoCoreServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/UmbracoCoreServiceCollectionExtensions.cs @@ -177,14 +177,11 @@ namespace Umbraco.Web.Common.Extensions // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured var serviceProvider = services.BuildServiceProvider(); - var httpContextAccessor = serviceProvider.GetRequiredService(); - configs = serviceProvider.GetService(); if (configs == null) throw new InvalidOperationException($"Could not resolve type {typeof(Configs)} from the container, ensure {nameof(AddUmbracoConfiguration)} is called before calling {nameof(AddUmbracoCore)}"); var hostingSettings = configs.Hosting(); - var coreDebug = configs.CoreDebug(); var globalSettings = configs.Global(); hostingEnvironment = new AspNetCoreHostingEnvironment(hostingSettings, webHostEnvironment); @@ -192,7 +189,7 @@ namespace Umbraco.Web.Common.Extensions logger = AddLogger(services, hostingEnvironment, loggingConfiguration); backOfficeInfo = new AspNetCoreBackOfficeInfo(globalSettings); - profiler = GetWebProfiler(hostingEnvironment, httpContextAccessor); + profiler = GetWebProfiler(hostingEnvironment); return services; } @@ -238,7 +235,7 @@ namespace Umbraco.Web.Common.Extensions return services; } - private static IProfiler GetWebProfiler(Umbraco.Core.Hosting.IHostingEnvironment hostingEnvironment, IHttpContextAccessor httpContextAccessor) + private static IProfiler GetWebProfiler(Umbraco.Core.Hosting.IHostingEnvironment hostingEnvironment) { // create and start asap to profile boot if (!hostingEnvironment.IsDebugMode) @@ -248,7 +245,7 @@ namespace Umbraco.Web.Common.Extensions return new VoidProfiler(); } - var webProfiler = new WebProfiler(httpContextAccessor); + var webProfiler = new WebProfiler(); webProfiler.StartBoot(); return webProfiler; diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs index f2034dbd82..0e2158c939 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs @@ -10,14 +10,19 @@ namespace Umbraco.Web.Common.Middleware public class UmbracoRequestLoggingMiddleware { readonly RequestDelegate _next; - private readonly ISessionIdResolver _sessionIdResolver; - private readonly IRequestCache _requestCache; + private readonly HttpSessionIdEnricher _sessionIdEnricher; + private readonly HttpRequestNumberEnricher _requestNumberEnricher; + private readonly HttpRequestIdEnricher _requestIdEnricher; - public UmbracoRequestLoggingMiddleware(RequestDelegate next, ISessionIdResolver sessionIdResolver, IRequestCache requestCache) + public UmbracoRequestLoggingMiddleware(RequestDelegate next, + HttpSessionIdEnricher sessionIdEnricher, + HttpRequestNumberEnricher requestNumberEnricher, + HttpRequestIdEnricher requestIdEnricher) { _next = next; - _sessionIdResolver = sessionIdResolver; - _requestCache = requestCache; + _sessionIdEnricher = sessionIdEnricher; + _requestNumberEnricher = requestNumberEnricher; + _requestIdEnricher = requestIdEnricher; } public async Task Invoke(HttpContext httpContext) @@ -25,9 +30,9 @@ namespace Umbraco.Web.Common.Middleware // TODO: Need to decide if we want this stuff still, there's new request logging in serilog: // https://github.com/serilog/serilog-aspnetcore#request-logging which i think would suffice and replace all of this? - using (LogContext.Push(new HttpSessionIdEnricher(_sessionIdResolver))) - using (LogContext.Push(new HttpRequestNumberEnricher(_requestCache))) - using (LogContext.Push(new HttpRequestIdEnricher(_requestCache))) + using (LogContext.Push(_sessionIdEnricher)) + using (LogContext.Push(_requestNumberEnricher)) + using (LogContext.Push(_requestIdEnricher)) { await _next(httpContext); } diff --git a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs index 79c7d3ec25..8af2824c03 100644 --- a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs +++ b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs @@ -7,6 +7,9 @@ using Umbraco.Core.Runtime; using Umbraco.Core.Security; using Umbraco.Web.Common.AspNetCore; using Umbraco.Web.Common.Lifetime; +using Umbraco.Core.Diagnostics; +using Umbraco.Web.Common.Runtime.Profiler; +using Umbraco.Core.Logging; namespace Umbraco.Web.Common.Runtime { @@ -42,6 +45,10 @@ namespace Umbraco.Web.Common.Runtime composition.RegisterUnique(); composition.RegisterMultipleUnique(); + + composition.RegisterUnique(); + + composition.RegisterUnique(); } } } diff --git a/src/Umbraco.Web.Common/Runtime/Profiler/WebProfiler.cs b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfiler.cs index bdbc6f164d..958e134bab 100644 --- a/src/Umbraco.Web.Common/Runtime/Profiler/WebProfiler.cs +++ b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfiler.cs @@ -1,61 +1,21 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using StackExchange.Profiling; -using StackExchange.Profiling.Internal; using Umbraco.Core; using Umbraco.Core.Logging; +// TODO: This namespace is strange, not sure why i has "Runtime" in the name? namespace Umbraco.Web.Common.Runtime.Profiler { + public class WebProfiler : IProfiler { private MiniProfiler _startupProfiler; - - private readonly IHttpContextAccessor _httpContextAccessor; private int _first; - public WebProfiler(IHttpContextAccessor httpContextAccessor) - { - // create our own provider, which can provide a profiler even during boot - _httpContextAccessor = httpContextAccessor; - } - - /// - /// - /// - /// - /// Normally we would call MiniProfiler.Current.RenderIncludes(...), but because the requeststate is not set, this method does not work. - /// We fake the requestIds from the RequestState here. - /// - public string Render() - { - - var profiler = MiniProfiler.Current; - if (profiler == null) return string.Empty; - - var context = _httpContextAccessor.HttpContext; - - var path = (profiler.Options as MiniProfilerOptions)?.RouteBasePath.Value.EnsureTrailingSlash(); - - var result = StackExchange.Profiling.Internal.Render.Includes( - profiler, - path: context.Request.PathBase + path, - isAuthorized: true, - requestIDs: new List{ profiler.Id }, - position: RenderPosition.Right, - showTrivial: profiler.Options.PopupShowTrivial, - showTimeWithChildren: profiler.Options.PopupShowTimeWithChildren, - maxTracesToShow: profiler.Options.PopupMaxTracesToShow, - showControls:profiler.Options.ShowControls, - startHidden: profiler.Options.PopupStartHidden); - - return result; - } - public IDisposable Step(string name) { return MiniProfiler.Current?.Step(name); diff --git a/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerComposer.cs b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerComposer.cs index 688a3e5c28..523faf2da5 100644 --- a/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerComposer.cs +++ b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerComposer.cs @@ -1,8 +1,15 @@ -using Umbraco.Core.Composing; +using Umbraco.Core; +using Umbraco.Core.Composing; namespace Umbraco.Web.Common.Runtime.Profiler { internal class WebProfilerComposer : ComponentComposer, ICoreComposer { + public override void Compose(Composition composition) + { + base.Compose(composition); + + composition.RegisterUnique(); + } } } diff --git a/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerHtml.cs b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerHtml.cs new file mode 100644 index 0000000000..9e989d6b5c --- /dev/null +++ b/src/Umbraco.Web.Common/Runtime/Profiler/WebProfilerHtml.cs @@ -0,0 +1,51 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using StackExchange.Profiling; +using StackExchange.Profiling.Internal; +using Umbraco.Core.Logging; + +// TODO: This namespace is strange, not sure why i has "Runtime" in the name? +namespace Umbraco.Web.Common.Runtime.Profiler +{ + public class WebProfilerHtml : IProfilerHtml + { + private readonly IHttpContextAccessor _httpContextAccessor; + + public WebProfilerHtml(IHttpContextAccessor httpContextAccessor) + { + // create our own provider, which can provide a profiler even during boot + _httpContextAccessor = httpContextAccessor; + } + + /// + /// + /// Normally we would call MiniProfiler.Current.RenderIncludes(...), but because the requeststate is not set, this method does not work. + /// We fake the requestIds from the RequestState here. + /// + public string Render() + { + + var profiler = MiniProfiler.Current; + if (profiler == null) return string.Empty; + + var context = _httpContextAccessor.HttpContext; + + var path = (profiler.Options as MiniProfilerOptions)?.RouteBasePath.Value.EnsureTrailingSlash(); + + var result = StackExchange.Profiling.Internal.Render.Includes( + profiler, + path: context.Request.PathBase + path, + isAuthorized: true, + requestIDs: new List { profiler.Id }, + position: RenderPosition.Right, + showTrivial: profiler.Options.PopupShowTrivial, + showTimeWithChildren: profiler.Options.PopupShowTimeWithChildren, + maxTracesToShow: profiler.Options.PopupMaxTracesToShow, + showControls: profiler.Options.ShowControls, + startHidden: profiler.Options.PopupStartHidden); + + return result; + } + } +} diff --git a/src/Umbraco.Web.UI.NetCore/Startup.cs b/src/Umbraco.Web.UI.NetCore/Startup.cs index d79fa0d917..8602d5bed6 100644 --- a/src/Umbraco.Web.UI.NetCore/Startup.cs +++ b/src/Umbraco.Web.UI.NetCore/Startup.cs @@ -91,7 +91,8 @@ namespace Umbraco.Web.UI.BackOffice }); endpoints.MapGet("/", async context => { - await context.Response.WriteAsync($"Hello World!{Current.Profiler.Render()}"); + var profilerHtml = app.ApplicationServices.GetRequiredService(); + await context.Response.WriteAsync($"Hello World!{profilerHtml.Render()}"); }); }); } diff --git a/src/Umbraco.Web/Composing/Current.cs b/src/Umbraco.Web/Composing/Current.cs index f9f056ff32..1ed217cc78 100644 --- a/src/Umbraco.Web/Composing/Current.cs +++ b/src/Umbraco.Web/Composing/Current.cs @@ -248,6 +248,8 @@ namespace Umbraco.Web.Composing public static IProfiler Profiler => Factory.GetInstance(); + public static IProfilerHtml ProfilerHtml => Factory.GetInstance(); + public static IProfilingLogger ProfilingLogger => Factory.GetInstance(); public static AppCaches AppCaches => Factory.GetInstance(); diff --git a/src/Umbraco.Web/HtmlHelperRenderExtensions.cs b/src/Umbraco.Web/HtmlHelperRenderExtensions.cs index 6c207dd15a..2795fe66c6 100644 --- a/src/Umbraco.Web/HtmlHelperRenderExtensions.cs +++ b/src/Umbraco.Web/HtmlHelperRenderExtensions.cs @@ -30,7 +30,7 @@ namespace Umbraco.Web /// public static IHtmlString RenderProfiler(this HtmlHelper helper) { - return new HtmlString(Current.Profiler.Render()); + return new HtmlString(Current.ProfilerHtml.Render()); } /// diff --git a/src/Umbraco.Web/Logging/WebProfiler.cs b/src/Umbraco.Web/Logging/WebProfiler.cs index e390950c0b..6f15be7ecd 100755 --- a/src/Umbraco.Web/Logging/WebProfiler.cs +++ b/src/Umbraco.Web/Logging/WebProfiler.cs @@ -14,7 +14,7 @@ namespace Umbraco.Web.Logging /// /// Profiling only runs when the app is in debug mode, see WebRuntime for how this gets created /// - internal class WebProfiler : IProfiler + internal class WebProfiler : IProfiler, IProfilerHtml { private const string BootRequestItemKey = "Umbraco.Core.Logging.WebProfiler__isBootRequest"; private readonly WebProfilerProvider _provider; diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index ec3b463d4c..5b59b632eb 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -25,6 +25,9 @@ using Umbraco.Web.Trees; using Umbraco.Web.WebApi; using Umbraco.Net; using Umbraco.Web.AspNet; +using Umbraco.Core.Diagnostics; +using Umbraco.Core.Logging; +using Umbraco.Web.Logging; namespace Umbraco.Web.Runtime { @@ -50,7 +53,8 @@ namespace Umbraco.Web.Runtime composition.Register(Lifetime.Singleton); - + composition.RegisterUnique(); + composition.RegisterUnique(); composition.ComposeWebMappingProfiles();