Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] ILogger integration #135

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
12 changes: 12 additions & 0 deletions samples/Samples.AspNetCore/Controllers/TestController.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using StackExchange.Exceptional;
using System;
using System.Threading.Tasks;
Expand All @@ -7,6 +8,17 @@ namespace Samples.AspNetCore.Controllers
{
public class TestController : Controller
{
private readonly ILogger<TestController> _logger;

public TestController(ILogger<TestController> logger) => _logger = logger;

public ActionResult Logger()
{
var ex = new Exception("Test Exception for ILogger goodness.");
_logger.LogError(ex, ex.Message);
return Content("Check the log!");
}

public async Task<ActionResult> Throw()
{
await ExceptionalUtils.Test.GetRedisException().LogAsync(ControllerContext.HttpContext).ConfigureAwait(false);
Expand Down
7 changes: 7 additions & 0 deletions samples/Samples.AspNetCore/Startup.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;

namespace Samples.AspNetCore
{
Expand All @@ -20,6 +22,11 @@ public Startup(IConfiguration configuration, IHostingEnvironment env)
public void ConfigureServices(IServiceCollection services)
{
services.AddMvc();
// (Optional): If you want ILogger calls that log an exception to have request details,
// then it needs access to the HttpContext statically, this registers that ability.
// If you're using Identity or ApplicationInsights, this is already registered.
// If using .NET Core 2.1+, you can call the new helper instead: services.AddHttpContextAccessor();
services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>();
// Make IOptions<ExceptionalSettings> available for injection everywhere
services.AddExceptional(Configuration.GetSection("Exceptional"), settings =>
{
Expand Down
1 change: 1 addition & 0 deletions samples/Samples.AspNetCore/Views/Shared/_Layout.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
<li><a asp-controller="Home" asp-action="Index">Home</a></li>
<li><a asp-controller="Home" asp-action="Exceptions">Exceptions</a></li>
<li><a asp-controller="Test" asp-action="Throw">Throw</a></li>
<li><a asp-controller="Test" asp-action="Logger">ILogger</a></li>
</ul>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using StackExchange.Exceptional;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using StackExchange.Exceptional;
using System;

namespace Microsoft.AspNetCore.Builder
Expand All @@ -16,7 +18,11 @@ public static class ExceptionalBuilderExtensions
public static IApplicationBuilder UseExceptional(this IApplicationBuilder builder)
{
_ = builder ?? throw new ArgumentNullException(nameof(builder));

var loggerFactory = builder.ApplicationServices.GetRequiredService<ILoggerFactory>();
loggerFactory.AddProvider(builder.ApplicationServices.GetRequiredService<ExceptionalLoggerProvider>());

return builder.UseMiddleware<ExceptionalMiddleware>();
}
}
}
}
52 changes: 52 additions & 0 deletions src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace StackExchange.Exceptional
{
internal class ExceptionalLogger : ILogger
{
private readonly string _category;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IOptions<ExceptionalSettings> _settings;

public ExceptionalLogger(string category, IOptions<ExceptionalSettings> settings, IHttpContextAccessor httpContextAccessor = null)
{
_category = category;
_settings = settings;
_httpContextAccessor = httpContextAccessor;
}

public IDisposable BeginScope<TState>(TState state) => null;

public bool IsEnabled(LogLevel logLevel) => _settings.Value.ILoggerLevel <= logLevel;

public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
{
// Ignore non-exceptions and Exceptional events themselves
if (exception == null || (ExceptionalLoggingEvents.Min <=eventId.Id && eventId.Id <= ExceptionalLoggingEvents.Max))
{
return;
}

var customData = new Dictionary<string, string>
{
["AspNetCore.LogLevel"] = logLevel.ToString(),
["AspNetCore.EventId.Id"] = eventId.Id.ToString(),
["AspNetCore.EventId.Name"] = eventId.Name,
["AspNetCore.Message"] = formatter(state, exception),
};

if (_httpContextAccessor?.HttpContext is HttpContext context)
{
exception.Log(context, _category, customData: customData);
}
else
{
exception.LogNoContext(_category, customData: customData);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

using System;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace StackExchange.Exceptional
{
internal class ExceptionalLoggerProvider : ILoggerProvider
{
private readonly IOptions<ExceptionalSettings> _settings;
private readonly IHttpContextAccessor _httpContextAccessor;

public ExceptionalLoggerProvider(IOptions<ExceptionalSettings> settings, IHttpContextAccessor httpContextAccessor = null)
{
_settings = settings;
_httpContextAccessor = httpContextAccessor;
}

ILogger ILoggerProvider.CreateLogger(string categoryName)
=> new ExceptionalLogger(categoryName, _settings, _httpContextAccessor);

void IDisposable.Dispose() { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace StackExchange.Exceptional
{
/// <summary>
/// Events IDs for known exceptional events while logging.
/// </summary>
public static class ExceptionalLoggingEvents
{
internal const int Min = RequestException;
internal const int Max = ExceptionalPageException;

/// <summary>
/// A request threw an exception, caught by Exceptional Middleware.
/// </summary>
public const int RequestException = 77000;

/// <summary>
/// A request was thrown while trying to render the Exceptional error page.
/// </summary>
public const int ExceptionalPageException = 77001;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public async Task Invoke(HttpContext context)
}
catch (Exception ex)
{
_logger.LogError(0, ex, "An unhandled exception has occurred, logging to Exceptional");
_logger.LogError(ExceptionalLoggingEvents.RequestException, ex, "An unhandled exception has occurred, logging to Exceptional");
var error = await ex.LogAsync(context).ConfigureAwait(false);

// If options say to do so, show the exception page to the user
Expand Down Expand Up @@ -101,7 +101,7 @@ public async Task Invoke(HttpContext context)
}
catch (Exception pex)
{
_logger.LogError(0, pex, "An exception was thrown attempting to display the Exceptional page.");
_logger.LogError(ExceptionalLoggingEvents.ExceptionalPageException, pex, "An exception was thrown attempting to display the Exceptional page.");
}
}
throw;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public static IServiceCollection AddExceptional(this IServiceCollection services
// When done configuring, set the background settings object for non-context logging.
services.Configure<ExceptionalSettings>(Exceptional.Configure);

// Setup for ILogger
services.AddSingleton<ExceptionalLoggerProvider>();

return services;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using StackExchange.Exceptional.Internal;
using System;

Expand All @@ -19,5 +20,11 @@ public class ExceptionalSettings : ExceptionalSettingsBase
/// but may need to be replaced in special multi-proxy situations.
/// </summary>
public Func<HttpContext, string> GetIPAddress { get; set; } = context => context.Connection.RemoteIpAddress.ToString();

/// <summary>
/// The minimum log level for <see cref="ExceptionalLogger"/>.
/// Defaults to <see cref="Microsoft.Extensions.Logging.LogLevel.Error"/>.
/// </summary>
public LogLevel ILoggerLevel { get; set; } = LogLevel.Error;
}
}
24 changes: 24 additions & 0 deletions src/StackExchange.Exceptional.Shared/Error.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ private void AddData(Exception exception)

foreach (string k in exception.Data.Keys)
{
if (k == Constants.LoggedDataKey)
{
continue;
}
if (regex?.IsMatch(k) == true)
{
InitCustomData();
Expand Down Expand Up @@ -201,8 +205,18 @@ public bool LogToStore(ErrorStore store = null)
if (abort) return false; // if we've been told to abort, then abort dammit!

Trace.WriteLine(Exception); // always echo the error to trace for local debugging

if (Exception.Data?.Contains(Constants.LoggedDataKey) == true)
{
// already logged - match the GUIDs up
GUID = (Guid)Exception.Data[Constants.LoggedDataKey];
return true;
}

store.Log(this);

if (Exception.Data != null) Exception.Data[Constants.LoggedDataKey] = GUID; // mark as logged

Settings.AfterLog(this, store);

return true;
Expand All @@ -220,8 +234,18 @@ public async Task<bool> LogToStoreAsync(ErrorStore store = null)
if (abort) return true; // if we've been told to abort, then abort dammit!

Trace.WriteLine(Exception); // always echo the error to trace for local debugging

if (Exception.Data?.Contains(Constants.LoggedDataKey) == true)
{
// already logged - match the GUIDs up
GUID = (Guid)Exception.Data[Constants.LoggedDataKey];
return true;
}

await store.LogAsync(this).ConfigureAwait(false);

if (Exception.Data != null) Exception.Data[Constants.LoggedDataKey] = GUID; // mark as logged

Settings.AfterLog(this, store);

return true;
Expand Down
5 changes: 5 additions & 0 deletions src/StackExchange.Exceptional.Shared/Internal/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,10 @@ public static class Constants
/// Key for prefixing fields in .Data for logging to CustomData
/// </summary>
public const string CustomDataKeyPrefix = "ExceptionalCustom-";

/// <summary>
/// The key in Exception.Data that indicates Exceptional has already handled this exception and should ignore future attempts to log it.
/// </summary>
public const string LoggedDataKey = "Exceptional.Logged";
}
}
63 changes: 63 additions & 0 deletions tests/StackExchange.Exceptional.Tests.AspNetCore/Logging.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
using StackExchange.Exceptional.Stores;
using Xunit;
using Xunit.Abstractions;

Expand Down Expand Up @@ -138,5 +146,60 @@ public async Task LogNoContext()
Assert.Null(error.ServerVariables);
}
}

[Fact]
public async Task ILoggerLogging()
{
Error error = null;
using (var server = new TestServer(new WebHostBuilder()
.ConfigureServices(services =>
{
services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>();
services.AddExceptional(s =>
{
s.DefaultStore = new MemoryErrorStore();
CurrentSettings = s;
});
})
.Configure(app =>
{
var logger = app.ApplicationServices.GetRequiredService<ILogger<Logging>>();
app.UseExceptional();
app.Run(async context =>
{
var ex = new Exception("Log!");
logger.LogError(ex, ex.Message);
var errors = await CurrentSettings.DefaultStore.GetAllAsync();
error = errors.FirstOrDefault();
await context.Response.WriteAsync("Hey.");
});
})))
{
using (var response = await server.CreateClient().GetAsync("?QueryKey=QueryValue").ConfigureAwait(false))
{
Assert.Equal("Hey.", await response.Content.ReadAsStringAsync().ConfigureAwait(false));
}

Assert.Equal("Log!", error.Message);
Assert.Equal("System.Exception", error.Type);
Assert.Equal(Environment.MachineName, error.MachineName);
Assert.Equal("localhost", error.Host);
Assert.Equal("http://localhost/?QueryKey=QueryValue", error.FullUrl);
Assert.Equal("/", error.UrlPath);

Assert.NotEmpty(error.RequestHeaders);
Assert.Equal("localhost", error.RequestHeaders["Host"]);

Assert.Single(error.QueryString);
Assert.Equal("QueryValue", error.QueryString["QueryKey"]);

Assert.NotEmpty(error.ServerVariables);
Assert.Equal("localhost", error.ServerVariables["Host"]);
Assert.Equal("/", error.ServerVariables["Path"]);
Assert.Equal("GET", error.ServerVariables["Request Method"]);
Assert.Equal("http", error.ServerVariables["Scheme"]);
Assert.Equal("http://localhost/?QueryKey=QueryValue", error.ServerVariables["Url"]);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<ProjectReference Include="..\..\src\StackExchange.Exceptional.MySQL\StackExchange.Exceptional.MySQL.csproj" />
<ProjectReference Include="..\..\src\StackExchange.Exceptional.PostgreSql\StackExchange.Exceptional.PostgreSql.csproj" />
<ProjectReference Include="..\..\src\StackExchange.Exceptional.MongoDB\StackExchange.Exceptional.MongoDB.csproj" />
<PackageReference Include="Jil" Version="2.15.4" />
<PackageReference Include="Jil" Version="2.16.0" />
</ItemGroup>

<ItemGroup>
Expand Down