-
Notifications
You must be signed in to change notification settings - Fork 165
[WIP] ILogger integration #135
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
Open
m0sa
wants to merge
13
commits into
NickCraver:main
Choose a base branch
from
m0sa:m0sa/aspnetcore-logging
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
6fa78e5
initial stab at ILogger integration, seems like we're getting duplica…
m0sa 73777dc
yo dawg, I've heard you like to log exceptions while you log exceptions
m0sa 2af7b78
Remove IHttpContextAccessor auto-registering
NickCraver 6791364
Add ILogger example to ASP.NET Core sample
NickCraver 5e3a119
Cleanup
NickCraver 2c4dd4b
ILogger: add setting for severity and known EventId entries
NickCraver 53aed8d
Fix comment
NickCraver 86893f7
Merge branch 'master' into pr/135
NickCraver 7c02a5d
Bump Jil for proper multi-platform testing
NickCraver 4e46c6a
Prevent duplicate logging fo cases like ILogger
NickCraver 58e2c47
Comment tips for ASP.NET Core
NickCraver cdb6329
ILogger: add test
NickCraver 302265c
Merge branch 'master' into pr/135
NickCraver File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.Extensions.Options; | ||
|
|
||
| namespace StackExchange.Exceptional | ||
| { | ||
| class ExceptionalLogger : ILogger | ||
| { | ||
| private readonly string _category; | ||
| private readonly IHttpContextAccessor _httpContextAccessor; | ||
| private readonly IOptions<ExceptionalSettings> _settings; | ||
| private readonly bool _ignored; | ||
| private static readonly HashSet<string> _ignoredCategories = new HashSet<string> | ||
| { | ||
| typeof(ExceptionalMiddleware).FullName, // exceptional middleware calls some ILogger stuff itself, ignore those calls) | ||
| }; | ||
|
|
||
| public ExceptionalLogger(string category, IOptions<ExceptionalSettings> settings, IHttpContextAccessor httpContextAccessor = null) | ||
| { | ||
| _category = category; | ||
| _settings = settings; | ||
| _httpContextAccessor = httpContextAccessor; | ||
| _ignored = _ignoredCategories.Contains(category); | ||
| } | ||
|
|
||
| public IDisposable BeginScope<TState>(TState state) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| public bool IsEnabled(LogLevel logLevel) | ||
| { | ||
| return !_ignored; | ||
| // TODO compare against settings and add support for per-category log levels, or we can just leave it up to LogFilters to decide later | ||
| } | ||
|
|
||
| public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) | ||
| { | ||
| if (_ignored) return; | ||
| if (exception == null) return; | ||
|
|
||
| var customData = new Dictionary<string, string> | ||
| { | ||
| ["AspNetCore.LogLevel"] = logLevel + "", | ||
| ["AspNetCore.EventId.Id"] = eventId.Id + "", | ||
| ["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); | ||
| } | ||
| } | ||
| } | ||
| } | ||
27 changes: 27 additions & 0 deletions
27
src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
|
|
||
| using System; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.Extensions.Options; | ||
|
|
||
| namespace StackExchange.Exceptional | ||
| { | ||
| 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() | ||
| { | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could attach something to Exception.Data, to prevent the same exception from being logged twice.
A reasonable case of this happening would be if an exception gets logged to an ILogger, is then rethrown, is ultimately captured by the middleware, and is logged again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran into exactly that while doing my own kinda version of this and doing that worked great: