From ad6f005130b3f4c01634e1ea7c498d143b87c3cb Mon Sep 17 00:00:00 2001 From: LucasZF Date: Tue, 1 Feb 2022 13:14:50 -0300 Subject: [PATCH] Reduced the logger noise from EF when not using Performance Monitoring (#1441) * Reduced the logger noise from EF when not using Performance Monitoring. Co-authored-by: Bruno Garcia --- CHANGELOG.md | 1 + .../DbInterceptionIntegration.cs | 25 ++++++++++++++--- .../SentryQueryPerformanceListener.cs | 5 ++-- .../DbInterceptionIntegrationTests.cs | 28 +++++++++++++++++++ .../SentryQueryPerformanceListenerTests.cs | 26 ++++++++++++++++- 5 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 test/Sentry.EntityFramework.Tests/DbInterceptionIntegrationTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 972345214f..c436c1f5c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Fixes +- Reduced the logger noise from EF when not using Performance Monitoring ([#1441](https://github.com/getsentry/sentry-dotnet/pull/1441)) - Create CachingTransport directories in constructor to avoid DirectoryNotFoundException ([#1432](https://github.com/getsentry/sentry-dotnet/pull/1432)) ## 3.13.0 diff --git a/src/Sentry.EntityFramework/DbInterceptionIntegration.cs b/src/Sentry.EntityFramework/DbInterceptionIntegration.cs index e53bc849c4..fd3bd9eeae 100644 --- a/src/Sentry.EntityFramework/DbInterceptionIntegration.cs +++ b/src/Sentry.EntityFramework/DbInterceptionIntegration.cs @@ -1,17 +1,34 @@ using System.Data.Entity.Infrastructure.Interception; +using Sentry.Extensibility; using Sentry.Integrations; namespace Sentry.EntityFramework; internal class DbInterceptionIntegration : ISdkIntegration { - private IDbInterceptor? _sqlInterceptor { get; set; } + // Internal for testing. + internal IDbInterceptor? SqlInterceptor { get; private set; } + + internal const string SampleRateDisabledMessage = "EF performance won't be collected because TracesSampleRate is set to 0."; public void Register(IHub hub, SentryOptions options) { - _sqlInterceptor = new SentryQueryPerformanceListener(hub, options); - DbInterception.Add(_sqlInterceptor); + if (options.TracesSampleRate == 0) + { + options.DiagnosticLogger?.LogInfo(SampleRateDisabledMessage); + } + else + { + SqlInterceptor = new SentryQueryPerformanceListener(hub, options); + DbInterception.Add(SqlInterceptor); + } } - public void Unregister() => DbInterception.Remove(_sqlInterceptor); + public void Unregister() + { + if (SqlInterceptor is { } interceptor) + { + DbInterception.Remove(interceptor); + } + } } diff --git a/src/Sentry.EntityFramework/SentryQueryPerformanceListener.cs b/src/Sentry.EntityFramework/SentryQueryPerformanceListener.cs index dd02ae9e60..959cfbfd0d 100644 --- a/src/Sentry.EntityFramework/SentryQueryPerformanceListener.cs +++ b/src/Sentry.EntityFramework/SentryQueryPerformanceListener.cs @@ -68,9 +68,10 @@ private void Finish(string key, DbCommandInterceptionContext interceptionC span.Finish(interceptionContext.Exception); } } - else + //Only log if there was a transaction on the Hub. + else if (_options.DiagnosticLevel == SentryLevel.Debug && _hub.GetSpan() is { }) { - _options.DiagnosticLogger?.LogWarning("Span with key {0} was not found on interceptionContext.", key); + _options.DiagnosticLogger?.LogDebug("Span with key {0} was not found on interceptionContext.", key); } } } diff --git a/test/Sentry.EntityFramework.Tests/DbInterceptionIntegrationTests.cs b/test/Sentry.EntityFramework.Tests/DbInterceptionIntegrationTests.cs new file mode 100644 index 0000000000..fbf226029e --- /dev/null +++ b/test/Sentry.EntityFramework.Tests/DbInterceptionIntegrationTests.cs @@ -0,0 +1,28 @@ +using Sentry.Testing; + +namespace Sentry.EntityFramework.Tests +{ + public class DbInterceptionIntegrationTests + { + [Fact] + public void Register_TraceSAmpleRateZero_IntegrationNotRegistered() + { + // Arrange + var logger = Substitute.For(); + var options = new SentryOptions() + { + Debug = true, + DiagnosticLogger = new TestOutputDiagnosticLogger(logger, SentryLevel.Debug), + TracesSampleRate = 0 + }; + var integration = new DbInterceptionIntegration(); + + // Act + integration.Register(Substitute.For(), options); + + // Assert + logger.Received(1).WriteLine(Arg.Is(message => message.Contains(DbInterceptionIntegration.SampleRateDisabledMessage))); + Assert.Null(integration.SqlInterceptor); + } + } +} diff --git a/test/Sentry.EntityFramework.Tests/SentryQueryPerformanceListenerTests.cs b/test/Sentry.EntityFramework.Tests/SentryQueryPerformanceListenerTests.cs index a8688133a7..03d2c0c8d8 100644 --- a/test/Sentry.EntityFramework.Tests/SentryQueryPerformanceListenerTests.cs +++ b/test/Sentry.EntityFramework.Tests/SentryQueryPerformanceListenerTests.cs @@ -2,6 +2,7 @@ using System.Data.Common; using System.Data.Entity.Infrastructure.Interception; using Effort.Provider; +using Sentry.Testing; namespace Sentry.EntityFramework.Tests; @@ -199,7 +200,7 @@ public void FirstOrDefault_FromDatabase_CapturesQuery() { // Arrange var integration = new DbInterceptionIntegration(); - integration.Register(_fixture.Hub, new SentryOptions()); + integration.Register(_fixture.Hub, new SentryOptions() { TracesSampleRate = 1 }); // Act _ = _fixture.DbContext.TestTable.FirstOrDefault(); @@ -221,4 +222,27 @@ public void FirstOrDefault_FromDatabase_CapturesQuery() }); integration.Unregister(); } + + [Fact] + public void Finish_NoActiveTransaction_LoggerNotCalled() + { + // Arrange + var hub = _fixture.Hub; + hub.GetSpan().Returns((_) => null); + var logger = Substitute.For(); + + var options = new SentryOptions() + { + Debug = true, + DiagnosticLogger = new TestOutputDiagnosticLogger(logger, SentryLevel.Debug) + }; + + var listener = new SentryQueryPerformanceListener(hub, options); + + // Act + listener.ScalarExecuted(Substitute.For(), Substitute.For>()); + + // Assert + logger.Received(0).WriteLine(Arg.Any()); + } }