-
-
Notifications
You must be signed in to change notification settings - Fork 225
feat(logs): Logs for Sentry.Extensions.Logging and integrations for Sentry.AspNetCore and Sentry.Maui
#4193
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
Changes from all commits
0add668
db7d558
d96b092
2958a47
a63371b
d8d2567
fefd24c
165996a
32e7e25
fc88722
2ba87e4
0f1d4a4
8dec5d5
a664f7e
96693d0
83964cf
dadc69b
c91cdde
0740c3b
eee06bf
8c61d8b
80683ae
cb20118
2cb306f
dcc0ec1
58dce74
f2e1ba2
0220015
6822b23
dd39fae
6eb5b9b
69c05b8
430cf82
31a8f1f
2ae4476
69678ce
97995a8
fbe747d
64adf33
cdfa901
d2ac53b
4011ba6
4ae82d0
bc1c465
79fb190
b4e80f4
b21adef
0032858
a9769f8
9a51033
8d03449
1bc3a6f
7624baa
11fe02b
fd532ca
1fe9cc0
476cbc3
9a09832
d9ce523
3e6dba5
23934dc
f133118
0985a76
72c9a93
c97f4ad
a9eea90
8bd0ed2
51892de
acc8995
6bd4c96
f673d1e
daafd7f
62ee5d5
6a54203
479cab8
f59160b
18a8284
97a87f8
0467449
54062d2
7107bce
c0a1cd5
45b8687
3192534
b8bcea6
d4c82a2
9193a96
7cb4043
e3ca5b5
afb135e
7f675aa
750a388
9f62d3b
5b00c21
6d17918
7e2c57b
7115c42
cd5246b
baf5569
e592d03
6e13e95
0a9a3b1
934fb36
2c1608e
2b09a79
2950be9
d428163
a936ec6
0fae148
8fc7a23
459e45d
92698dd
1277a2f
5a880e0
d6083bd
353a8de
0de2ba9
bef0de6
9463aa7
8a89441
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| ``` | ||
|
|
||
| BenchmarkDotNet v0.13.12, macOS 15.5 (24F74) [Darwin 24.5.0] | ||
| Apple M3 Pro, 1 CPU, 12 logical and 12 physical cores | ||
| .NET SDK 9.0.301 | ||
| [Host] : .NET 8.0.14 (8.0.1425.11118), Arm64 RyuJIT AdvSIMD | ||
| DefaultJob : .NET 8.0.14 (8.0.1425.11118), Arm64 RyuJIT AdvSIMD | ||
|
|
||
|
|
||
| ``` | ||
| | Method | Mean | Error | StdDev | Gen0 | Allocated | | ||
| |------- |---------:|--------:|--------:|-------:|----------:| | ||
| | Log | 288.4 ns | 1.28 ns | 1.20 ns | 0.1163 | 976 B | |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| #nullable enable | ||
|
|
||
| using BenchmarkDotNet.Attributes; | ||
| using Microsoft.Extensions.Logging; | ||
| using Sentry.Extensibility; | ||
| using Sentry.Extensions.Logging; | ||
| using Sentry.Internal; | ||
| using Sentry.Testing; | ||
|
|
||
| namespace Sentry.Benchmarks.Extensions.Logging; | ||
|
|
||
| public class SentryStructuredLoggerBenchmarks | ||
| { | ||
| private Hub _hub = null!; | ||
| private Sentry.Extensions.Logging.SentryStructuredLogger _logger = null!; | ||
| private LogRecord _logRecord = null!; | ||
| private SentryLog? _lastLog; | ||
|
|
||
| [GlobalSetup] | ||
| public void Setup() | ||
| { | ||
| SentryLoggingOptions options = new() | ||
| { | ||
| Dsn = DsnSamples.ValidDsn, | ||
| Experimental = | ||
| { | ||
| EnableLogs = true, | ||
| }, | ||
| ExperimentalLogging = | ||
| { | ||
| MinimumLogLevel = LogLevel.Information, | ||
| } | ||
| }; | ||
| options.Experimental.SetBeforeSendLog((SentryLog log) => | ||
| { | ||
| _lastLog = log; | ||
| return null; | ||
| }); | ||
|
|
||
| MockClock clock = new(new DateTimeOffset(2025, 04, 22, 14, 51, 00, 789, TimeSpan.FromHours(2))); | ||
| SdkVersion sdk = new() | ||
| { | ||
| Name = "SDK Name", | ||
| Version = "SDK Version", | ||
| }; | ||
|
|
||
| _hub = new Hub(options, DisabledHub.Instance); | ||
| _logger = new Sentry.Extensions.Logging.SentryStructuredLogger("CategoryName", options, _hub, clock, sdk); | ||
| _logRecord = new LogRecord(LogLevel.Information, new EventId(2025, "EventName"), new InvalidOperationException("exception-message"), "Number={Number}, Text={Text}", 2018, "message"); | ||
| } | ||
|
|
||
| [Benchmark] | ||
| public void Log() | ||
| { | ||
| _logger.Log(_logRecord.LogLevel, _logRecord.EventId, _logRecord.Exception, _logRecord.Message, _logRecord.Args); | ||
| } | ||
|
|
||
| [GlobalCleanup] | ||
| public void Cleanup() | ||
| { | ||
| _hub.Dispose(); | ||
|
|
||
| if (_lastLog is null) | ||
| { | ||
| throw new InvalidOperationException("Last Log is null"); | ||
| } | ||
| if (_lastLog.Message != "Number=2018, Text=message") | ||
| { | ||
| throw new InvalidOperationException($"Last Log with Message: '{_lastLog.Message}'"); | ||
| } | ||
| } | ||
|
|
||
| private sealed class LogRecord | ||
| { | ||
| public LogRecord(LogLevel logLevel, EventId eventId, Exception? exception, string? message, params object?[] args) | ||
| { | ||
| LogLevel = logLevel; | ||
| EventId = eventId; | ||
| Exception = exception; | ||
| Message = message; | ||
| Args = args; | ||
| } | ||
|
|
||
| public LogLevel LogLevel { get; } | ||
| public EventId EventId { get; } | ||
| public Exception? Exception { get; } | ||
| public string? Message { get; } | ||
| public object?[] Args { get; } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.Extensions.Options; | ||
| using Sentry.Extensions.Logging; | ||
| using Sentry.Infrastructure; | ||
|
|
||
| namespace Sentry.AspNetCore; | ||
|
|
||
| /// <summary> | ||
| /// Structured Logger Provider for Sentry. | ||
| /// </summary> | ||
| [ProviderAlias("SentryLogs")] | ||
| internal sealed class SentryAspNetCoreStructuredLoggerProvider : SentryStructuredLoggerProvider | ||
| { | ||
| public SentryAspNetCoreStructuredLoggerProvider(IOptions<SentryAspNetCoreOptions> options, IHub hub) | ||
| : this(options.Value, hub, SystemClock.Clock, CreateSdkVersion()) | ||
| { | ||
| } | ||
|
|
||
| internal SentryAspNetCoreStructuredLoggerProvider(SentryAspNetCoreOptions options, IHub hub, ISystemClock clock, SdkVersion sdk) | ||
| : base(options, hub, clock, sdk) | ||
| { | ||
| } | ||
|
|
||
| private static SdkVersion CreateSdkVersion() | ||
| { | ||
| return new SdkVersion | ||
| { | ||
| Name = Constants.SdkName, | ||
| Version = SentryMiddleware.NameAndVersion.Version, | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,4 +45,19 @@ public static SentryLevel ToSentryLevel(this LogLevel level) | |
| _ => SentryLevel.Debug | ||
| }; | ||
| } | ||
|
|
||
| public static SentryLogLevel ToSentryLogLevel(this LogLevel logLevel) | ||
| { | ||
| return logLevel switch | ||
| { | ||
| LogLevel.Trace => SentryLogLevel.Trace, | ||
| LogLevel.Debug => SentryLogLevel.Debug, | ||
| LogLevel.Information => SentryLogLevel.Info, | ||
| LogLevel.Warning => SentryLogLevel.Warning, | ||
| LogLevel.Error => SentryLogLevel.Error, | ||
| LogLevel.Critical => SentryLogLevel.Fatal, | ||
| LogLevel.None => default, | ||
| _ => default, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. potentially constructed scenario, but possible logger.Log((LogLevel)-1, "message");
logger.Log((LogLevel)7, "message");What should happen? |
||
| }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Invalid SentryLogLevel for Unhandled LogLevelsThe |
||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Invalid SentryLogLevel for Unhandled LogLevelsThe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Invalid SentryLogLevel for LogLevel.NoneThe |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,9 @@ public class SentryLoggingOptions : SentryOptions | |
| /// <summary> | ||
| /// Gets or sets the minimum breadcrumb level. | ||
| /// </summary> | ||
| /// <remarks>Events with this level or higher will be stored as <see cref="Breadcrumb"/></remarks> | ||
| /// <remarks> | ||
| /// Events with this level or higher will be stored as <see cref="Breadcrumb"/>. | ||
| /// </remarks> | ||
| /// <value> | ||
| /// The minimum breadcrumb level. | ||
| /// </value> | ||
|
|
@@ -21,7 +23,7 @@ public class SentryLoggingOptions : SentryOptions | |
| /// Gets or sets the minimum event level. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Events with this level or higher will be sent to Sentry | ||
| /// Events with this level or higher will be sent to Sentry. | ||
| /// </remarks> | ||
| /// <value> | ||
| /// The minimum event level. | ||
|
|
@@ -48,4 +50,39 @@ public class SentryLoggingOptions : SentryOptions | |
| /// List of callbacks to be invoked when initializing the SDK | ||
| /// </summary> | ||
| internal Action<Scope>[] ConfigureScopeCallbacks { get; set; } = Array.Empty<Action<Scope>>(); | ||
|
|
||
| /// <summary> | ||
| /// Experimental Sentry Logging features. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This and related experimental APIs may change in the future. | ||
| /// </remarks> | ||
| [Experimental(Infrastructure.DiagnosticId.ExperimentalFeature)] | ||
| public SentryLoggingExperimentalOptions ExperimentalLogging { get; set; } = new(); | ||
|
|
||
| /// <summary> | ||
| /// Experimental Sentry Logging options. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This and related experimental APIs may change in the future. | ||
| /// </remarks> | ||
| [Experimental(Infrastructure.DiagnosticId.ExperimentalFeature)] | ||
| public sealed class SentryLoggingExperimentalOptions | ||
| { | ||
| internal SentryLoggingExperimentalOptions() | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the minimum log level. | ||
|
Comment on lines
+76
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default minimum log level is set to Did we get this right? 👍 / 👎 to inform future reviews. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point. The spec/developer docs, see https://develop.sentry.dev/sdk/telemetry/logs/, mention:
Per default, no logs are sent to Sentry, regardless of the @jamescrosswell @bruno-garcia SentryOptions.Experimental.EnableLogs = false;
SentryLoggingOptions.ExperimentalLogging.MinimumLogLevel = LogLevel.Trace;which means that if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, that could be quite verbose. MEL let's you configure different log levels for different categories as well... so eventually we might want to mimic/reuse that configuration. |
||
| /// <para>This API is experimental and it may change in the future.</para> | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Logs with this level or higher will be stored as <see cref="SentryLog"/>. | ||
| /// </remarks> | ||
| /// <value> | ||
| /// The minimum log level. | ||
| /// </value> | ||
| public LogLevel MinimumLogLevel { get; set; } = LogLevel.Trace; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: rename to also ensure XML docs mention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider: removing this option altogether There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will remove in upcoming changeset |
||
| } | ||
| } | ||
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.
are these guaranteed to be all of the categories we add?
I wonder if there's some "Constant" or something with these, we could reflect on and write a test to make sure we filter all of them out
Uh oh!
There was an error while loading. Please reload this page.
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'm fairly certain ... not quite positive though.
Those are the
ILogger<T>usages that I found in our packages.I definitely have to add a test for that ... probably as an ASP.NET Core integration test ... I see we have the infrastructure already setup via the "TestUtils" project.
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.
There's this one (although admittedly it's not used):
sentry-dotnet/src/Sentry.AspNetCore/RequestDecompression/RequestDecompressionMiddleware.cs
Line 33 in c5db091
btw: Why an integration test for this? It just needs to use reflection to iterate over a bunch of assemblies right? Should be possible in a unit test?