diff --git a/.craft.yml b/.craft.yml index 535f6ad22c..d098d717f6 100644 --- a/.craft.yml +++ b/.craft.yml @@ -19,6 +19,7 @@ targets: nuget:Sentry.Hangfire: nuget:Sentry.Log4Net: nuget:Sentry.Maui: + nuget:Sentry.Maui.CommunityToolkit.Mvvm: nuget:Sentry.NLog: nuget:Sentry.OpenTelemetry: nuget:Sentry.OpenTelemetry.Exporter: diff --git a/src/Sentry.Extensions.Logging/SentryLogger.cs b/src/Sentry.Extensions.Logging/SentryLogger.cs index 32394b52b7..65eb1ff4da 100644 --- a/src/Sentry.Extensions.Logging/SentryLogger.cs +++ b/src/Sentry.Extensions.Logging/SentryLogger.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Logging; using Sentry.Infrastructure; +using Sentry.Internal; namespace Sentry.Extensions.Logging; @@ -182,22 +183,7 @@ private bool ShouldAddBreadcrumb( exception)); - private bool IsFromSentry() - { - if (string.Equals(CategoryName, "Sentry", StringComparison.Ordinal)) - { - return true; - } - -#if DEBUG - if (CategoryName.StartsWith("Sentry.Samples.", StringComparison.Ordinal)) - { - return false; - } -#endif - - return CategoryName.StartsWith("Sentry.", StringComparison.Ordinal); - } + private bool IsFromSentry() => SentrySdkNamespaces.IsSentrySdk(CategoryName); internal static bool IsEfExceptionMessage(EventId eventId) { diff --git a/src/Sentry.Serilog/Sentry.Serilog.csproj b/src/Sentry.Serilog/Sentry.Serilog.csproj index 8a5acc19ba..c49ce23f98 100644 --- a/src/Sentry.Serilog/Sentry.Serilog.csproj +++ b/src/Sentry.Serilog/Sentry.Serilog.csproj @@ -19,6 +19,7 @@ + diff --git a/src/Sentry.Serilog/SentrySink.cs b/src/Sentry.Serilog/SentrySink.cs index f00556794b..8076d2da6d 100644 --- a/src/Sentry.Serilog/SentrySink.cs +++ b/src/Sentry.Serilog/SentrySink.cs @@ -91,7 +91,7 @@ private void InnerEmit(LogEvent logEvent) { if (logEvent.TryGetSourceContext(out var context)) { - if (IsSentryContext(context)) + if (SentrySdkNamespaces.IsSentrySdk(context)) { return; } @@ -175,10 +175,6 @@ private void InnerEmit(LogEvent logEvent) } } - private static bool IsSentryContext(string context) => - context.StartsWith("Sentry.") || - string.Equals(context, "Sentry", StringComparison.Ordinal); - private string FormatLogEvent(LogEvent logEvent) { if (_options.TextFormatter is { } formatter) diff --git a/src/Sentry/Internal/SentrySdkNamespaces.cs b/src/Sentry/Internal/SentrySdkNamespaces.cs new file mode 100644 index 0000000000..b898cc8543 --- /dev/null +++ b/src/Sentry/Internal/SentrySdkNamespaces.cs @@ -0,0 +1,61 @@ +namespace Sentry.Internal; + +/// +/// Identifies logger names that belong to the Sentry SDK itself, so the logging integrations can +/// skip them rather than feeding the SDK's own output back into Sentry. +/// +/// +/// The PackageRoots half of this class is generated from .craft.yml - see the +/// GenerateSentrySdkNamespaces target in Sentry.csproj. +/// +internal static partial class SentrySdkNamespaces +{ + private const string Prefix = "Sentry"; + + /// + /// The core Sentry package can't be matched by prefix the way the other packages can: + /// all of its types live directly under Sentry., and so does application code that + /// happens to use that namespace (Sentry.Samples.*, Sentry.MyApp.*). Matching on + /// Sentry. would therefore silently discard the user's own logs. + /// + /// Core has no logging dependency of its own, so the only name it ever logs under is the one + /// MelDiagnosticLogger (in Sentry.Extensions.Logging) gets from + /// ILogger<ISentryClient>. Matching that exactly is enough. + /// + private static readonly string[] CoreNames = { Prefix, "Sentry.ISentryClient" }; + + /// + /// Whether - a Serilog SourceContext, a + /// Microsoft.Extensions.Logging category, or an equivalent from another framework - + /// belongs to the Sentry SDK. + /// + internal static bool IsSentrySdk(string? loggerName) + { + if (loggerName is null || !loggerName.StartsWith(Prefix, StringComparison.Ordinal)) + { + return false; + } + + foreach (var name in CoreNames) + { + if (string.Equals(loggerName, name, StringComparison.Ordinal)) + { + return true; + } + } + + foreach (var root in PackageRoots) + { + // Either the package's own namespace, or something nested inside it. Comparing the + // character after the root keeps a package named e.g. 'Sentry.Maui' from matching an + // unrelated 'Sentry.MauiSomething'. + if (loggerName.StartsWith(root, StringComparison.Ordinal) && + (loggerName.Length == root.Length || loggerName[root.Length] == '.')) + { + return true; + } + } + + return false; + } +} diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 3684ad5862..f8b5a34f4c 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -225,4 +225,72 @@ + + + <_SentryCraftManifest>$(MSBuildThisFileDirectory)..\..\.craft.yml + + + + + + + + + + + <_SentrySdkPackage Include="$([System.Text.RegularExpressions.Regex]::Replace('%(_SentryCraftLine.Identity)', '^\s*nuget:([A-Za-z0-9\.]+):\s*$', '$1'))" + Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('%(_SentryCraftLine.Identity)', '^\s*nuget:[A-Za-z0-9\.]+:\s*$'))" /> + + <_SentrySdkPackage Remove="Sentry" /> + + + + + + <_SentrySdkNamespacesLine Include="// <auto-generated/>" /> + <_SentrySdkNamespacesLine Include="// Generated from .craft.yml by the GenerateSentrySdkNamespaces target in Sentry.csproj." /> + <_SentrySdkNamespacesLine Include="namespace Sentry.Internal%3B" /> + <_SentrySdkNamespacesLine Include="internal static partial class SentrySdkNamespaces" /> + <_SentrySdkNamespacesLine Include="{" /> + <_SentrySdkNamespacesLine Include=" // Namespace roots owned by SDK packages other than the core 'Sentry' package." /> + <_SentrySdkNamespacesLine Include=" internal static readonly string[] PackageRoots =" /> + <_SentrySdkNamespacesLine Include=" {" /> + <_SentrySdkNamespacesLine Include=" "%(_SentrySdkPackage.Identity)"," /> + <_SentrySdkNamespacesLine Include=" }%3B" /> + <_SentrySdkNamespacesLine Include="}" /> + + + + + + + + + + + + + diff --git a/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs b/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs index baf89cee44..02f2bf9531 100644 --- a/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs +++ b/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs @@ -218,24 +218,15 @@ public void LogError_DefaultOptions_CapturesEvent() .CaptureEvent(Arg.Any()); } - [Fact] - public void Log_SentryCategory_DoesNotSendEvent() + [Theory] + [InlineData("Sentry")] + [InlineData("Sentry.ISentryClient")] + [InlineData("Sentry.Extensions.Logging.SentryLogger")] + [InlineData("Sentry.AspNetCore.SentryMiddleware")] + public void Log_SentryCategory_DoesNotSendEvent(string categoryName) { var expectedException = new Exception("expected message"); - _fixture.CategoryName = "Sentry.Some.Class"; - var sut = _fixture.GetSut(); - - sut.Log(LogLevel.Critical, default, null, expectedException, null); - - _ = _fixture.Hub.DidNotReceive() - .CaptureEvent(Arg.Any()); - } - - [Fact] - public void Log_SentryRootCategory_DoesNotSendEvent() - { - var expectedException = new Exception("expected message"); - _fixture.CategoryName = "Sentry"; + _fixture.CategoryName = categoryName; var sut = _fixture.GetSut(); sut.Log(LogLevel.Critical, default, null, expectedException, null); @@ -244,12 +235,17 @@ public void Log_SentryRootCategory_DoesNotSendEvent() .CaptureEvent(Arg.Any()); } + // A category under a "Sentry" namespace that isn't one of ours belongs to the application. // https://github.com/getsentry/sentry-dotnet/issues/132 - [Fact] - public void Log_SentrySomethingCategory_SendEvent() + // https://github.com/getsentry/sentry-dotnet/issues/5265 + [Theory] + [InlineData("SentrySomething")] + [InlineData("Sentry.Some.Class")] + [InlineData("Sentry.Samples.AspNetCore.Serilog.Program")] + public void Log_UserCategory_SendsEvent(string categoryName) { var expectedException = new Exception("expected message"); - _fixture.CategoryName = "SentrySomething"; + _fixture.CategoryName = categoryName; var sut = _fixture.GetSut(); sut.Log(LogLevel.Critical, default, null, expectedException, null); diff --git a/test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs b/test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs index 3c2c9798b8..a54af7394d 100644 --- a/test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs +++ b/test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs @@ -71,5 +71,46 @@ public async Task StructuredLogging_Enabled() Assert.NotEmpty(Logs); Assert.Contains(Logs, log => log.Level == SentryLogLevel.Info && log.Message == "Hello, World!"); } + + [Fact] + public async Task ILoggerFromApplicationNamespaceStartingWithSentry_CapturesEventAndRunsEventProcessors() + { + // Logs from our samples shouldn't be filtered - only logs from our SDK should be + const string category = "Sentry.Samples.AspNetCore.Serilog.Program"; + + var processor = new RecordingEventProcessor(); + ConfigureServices = services => services.AddSingleton(processor); + + var handler = new RequestHandler + { + Path = "/log", + Handler = context => + { + context.RequestServices.GetRequiredService() + .CreateLogger(category) + .LogError(new InvalidOperationException("hello?"), "This is a problem"); + return Task.CompletedTask; + } + }; + + Handlers = new[] { handler }; + Build(); + await HttpClient.GetAsync(handler.Path); + await ServiceProvider.GetRequiredService().FlushAsync(); + + Assert.Contains(Events, e => e.Logger == category); + Assert.True(processor.Invoked); + } + + private class RecordingEventProcessor : ISentryEventProcessor + { + public bool Invoked { get; private set; } + + public SentryEvent Process(SentryEvent @event) + { + Invoked = true; + return @event; + } + } } #endif diff --git a/test/Sentry.Serilog.Tests/SentrySinkTests.cs b/test/Sentry.Serilog.Tests/SentrySinkTests.cs index 0ed6e94139..18ce0a4561 100644 --- a/test/Sentry.Serilog.Tests/SentrySinkTests.cs +++ b/test/Sentry.Serilog.Tests/SentrySinkTests.cs @@ -281,60 +281,60 @@ public void Emit_WithTextFormatter_EventCaptured() && p.Message.Message == expectedMessage)); } - [Fact] - public void Emit_SourceContextMatchesSentry_NoEventSent() - { - var sut = _fixture.GetSut(); - - var evt = new LogEvent(DateTimeOffset.UtcNow, LogEventLevel.Error, null, - new MessageTemplateParser().Parse("message"), - new[] { new LogEventProperty("SourceContext", new ScalarValue("Sentry.Serilog")) }); - - sut.Emit(evt); - - _fixture.Hub.DidNotReceive().CaptureEvent(Arg.Any()); - } - - [Fact] - public void Emit_SourceContextContainsSentry_NoEventSent() + [Theory] + [InlineData("Sentry")] + [InlineData("Sentry.ISentryClient")] + [InlineData("Sentry.Serilog")] + [InlineData("Sentry.Serilog.SentrySink")] + [InlineData("Sentry.AspNetCore.SentryMiddleware")] + [InlineData("Sentry.Extensions.Logging.MelDiagnosticLogger")] + public void Emit_SourceContextFromSentrySdk_NoEventSent(string sourceContext) { var sut = _fixture.GetSut(); var evt = new LogEvent(DateTimeOffset.UtcNow, LogEventLevel.Error, null, new MessageTemplateParser().Parse("message"), - new[] { new LogEventProperty("SourceContext", new ScalarValue("Sentry")) }); + new[] { new LogEventProperty("SourceContext", new ScalarValue(sourceContext)) }); sut.Emit(evt); _fixture.Hub.DidNotReceive().CaptureEvent(Arg.Any()); } - [Fact] - public void Emit_SourceContextMatchesSentry_NoScopeConfigured() + [Theory] + [InlineData("Sentry")] + [InlineData("Sentry.ISentryClient")] + [InlineData("Sentry.Serilog")] + [InlineData("Sentry.AspNetCore.SentryMiddleware")] + public void Emit_SourceContextFromSentrySdk_NoScopeConfigured(string sourceContext) { var sut = _fixture.GetSut(); var evt = new LogEvent(DateTimeOffset.UtcNow, LogEventLevel.Error, null, new MessageTemplateParser().Parse("message"), - new[] { new LogEventProperty("SourceContext", new ScalarValue("Sentry.Serilog")) }); + new[] { new LogEventProperty("SourceContext", new ScalarValue(sourceContext)) }); sut.Emit(evt); _fixture.Hub.DidNotReceive().ConfigureScope(Arg.Any>()); } - [Fact] - public void Emit_SourceContextContainsSentry_NoScopeConfigured() + // See https://github.com/getsentry/sentry-dotnet/issues/5265 + [Theory] + [InlineData("Sentry.Samples.AspNetCore.Serilog.Program")] + [InlineData("Sentry.Some.Class")] + [InlineData("SentrySomething")] + public void Emit_SourceContextFromUserCode_EventSent(string sourceContext) { var sut = _fixture.GetSut(); var evt = new LogEvent(DateTimeOffset.UtcNow, LogEventLevel.Error, null, new MessageTemplateParser().Parse("message"), - new[] { new LogEventProperty("SourceContext", new ScalarValue("Sentry")) }); + new[] { new LogEventProperty("SourceContext", new ScalarValue(sourceContext)) }); sut.Emit(evt); - _fixture.Hub.DidNotReceive().ConfigureScope(Arg.Any>()); + _fixture.Hub.Received(1).CaptureEvent(Arg.Is(e => e.Logger == sourceContext)); } [Fact] diff --git a/test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs b/test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs new file mode 100644 index 0000000000..16a657b566 --- /dev/null +++ b/test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs @@ -0,0 +1,49 @@ +namespace Sentry.Tests.Internals; + +public class SentrySdkNamespacesTests +{ + [Theory] + [InlineData("Sentry")] + [InlineData("Sentry.ISentryClient")] + [InlineData("Sentry.AspNetCore")] + [InlineData("Sentry.AspNetCore.SentryMiddleware")] + [InlineData("Sentry.AspNetCore.RequestDecompression.RequestDecompressionMiddleware")] + [InlineData("Sentry.Extensions.Logging.MelDiagnosticLogger")] + [InlineData("Sentry.Serilog.SentrySink")] + [InlineData("Sentry.Maui.SentryMauiOptions")] + public void IsSentrySdk_SdkLoggerName_True(string loggerName) + => SentrySdkNamespaces.IsSentrySdk(loggerName).Should().BeTrue(); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("MyApp.Program")] + [InlineData("Microsoft.AspNetCore.Hosting.Diagnostics")] + // Logs from our samples shouldn't be filtered + [InlineData("Sentry.Samples.AspNetCore.Serilog.Program")] + [InlineData("Sentry.Samples.Console.Basic")] + [InlineData("Sentry.Some.Class")] + [InlineData("Sentry.MyApp.Worker")] + // Starts with Sentry but not one of our SDK namespaces + [InlineData("SentrySomething")] + // A package root must match on a namespace boundary, not just as a string prefix. + [InlineData("Sentry.MauiSomething.Thing")] + [InlineData("Sentry.AspNetCoreExtras.Thing")] + public void IsSentrySdk_NotAnSdkLoggerName_False(string loggerName) + => SentrySdkNamespaces.IsSentrySdk(loggerName).Should().BeFalse(); + + [Fact] + public void PackageRoots_GeneratedFromCraftManifest_ExcludesCoreAndCoversIntegrations() + { + // Guards the .craft.yml parsing in the GenerateSentrySdkNamespaces target: a silently empty + // or malformed list would stop the integrations recognising the SDK's own log messages. + SentrySdkNamespaces.PackageRoots.Should() + .NotBeEmpty() + .And.OnlyContain(root => root.StartsWith("Sentry.", StringComparison.Ordinal)) + .And.Contain("Sentry.AspNetCore") + .And.Contain("Sentry.Extensions.Logging") + .And.Contain("Sentry.Serilog") + // 'Sentry' itself cannot be a prefix root - see SentrySdkNamespaces.CoreNames. + .And.NotContain("Sentry"); + } +}