Cleanup diagnostic source

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Ignore zero properties for MemoryInfo ([#1531](https://github.com/getsentry/sentry-dotnet/pull/1531))
+- Cleanup diagnostic source ([#1529](https://github.com/getsentry/sentry-dotnet/pull/1529))
 
 ## 3.15.0
 

src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentryDiagnosticListenerIntegration.cs

@@ -1,26 +1,26 @@
 using System;
 using System.Diagnostics;
+using Sentry.Extensibility;
 using Sentry.Internal;
 
 namespace Sentry.Internals.DiagnosticSource
 {
     internal class SentryDiagnosticListenerIntegration : IInternalSdkIntegration
     {
         private SentryDiagnosticSubscriber? _subscriber;
-        private IDisposable? _diagnosticListener { get; set; }
+        private IDisposable? _diagnosticListener;
 
         public void Register(IHub hub, SentryOptions options)
         {
             if (options.TracesSampleRate == 0)
             {
-                options.DiagnosticLogger?.Log(SentryLevel.Info, "DiagnosticSource Integration is now disabled due to TracesSampleRate being set to zero.");
+                options.Log(SentryLevel.Info, "DiagnosticSource Integration is now disabled due to TracesSampleRate being set to zero.");
                 options.DisableDiagnosticSourceIntegration();
+                return;
             }
-            else
-            {
-                _subscriber = new SentryDiagnosticSubscriber(hub, options);
-                _diagnosticListener = DiagnosticListener.AllListeners.Subscribe(_subscriber);
-            }
+
+            _subscriber = new SentryDiagnosticSubscriber(hub, options);
+            _diagnosticListener = DiagnosticListener.AllListeners.Subscribe(_subscriber);
         }
 
         public void Unregister(IHub hub)

src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentryDiagnosticSubscriber.cs

@@ -10,11 +10,11 @@ namespace Sentry.Internals.DiagnosticSource
     /// </summary>
     internal class SentryDiagnosticSubscriber : IObserver<DiagnosticListener>, IDisposable
     {
-        private SentryEFCoreListener? _efInterceptor { get; set; }
-        private SentrySqlListener? _sqlListener { get; set; }
-        private ConcurrentBag<IDisposable> _disposableListeners = new();
-        private IHub _hub { get; }
-        private SentryOptions _options { get; }
+        private SentryEFCoreListener? _efInterceptor;
+        private SentrySqlListener? _sqlListener;
+        private readonly ConcurrentBag<IDisposable> _disposableListeners = new();
+        private readonly IHub _hub;
+        private readonly SentryOptions _options;
 
         public SentryDiagnosticSubscriber(IHub hub, SentryOptions options)
         {
@@ -33,8 +33,10 @@ public void OnNext(DiagnosticListener listener)
                 _efInterceptor = new(_hub, _options);
                 _disposableListeners.Add(listener.Subscribe(_efInterceptor));
                 _options.Log(SentryLevel.Debug, "Registered integration with EF Core.");
+                return;
             }
-            else if (listener.Name == "SqlClientDiagnosticListener")
+
+            if (listener.Name == "SqlClientDiagnosticListener")
             {
                 _sqlListener = new(_hub, _options);
                 _disposableListeners.Add(listener.Subscribe(_sqlListener));

src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentryEFCoreListener.cs

@@ -1,8 +1,6 @@
-#if NETCOREAPP3_0_OR_GREATER
-using System;
-using System.Collections.Generic;
 using System.Threading;
-#endif
+using System.Collections.Generic;
+using System;
 using Sentry.Extensibility;
 
 namespace Sentry.Internals.DiagnosticSource
@@ -27,22 +25,22 @@ private enum SentryEFSpanType
 
         /// <summary>
         /// Used for EF Core 2.X and 3.X.
-        /// <seealso href="https://docs.microsoft.com/dotnet/api/microsoft.entityframeworkcore.diagnostics.coreeventid.querymodelcompiling?view=efcore-3.1"></seealso>
+        /// <seealso href="https://docs.microsoft.com/dotnet/api/microsoft.entityframeworkcore.diagnostics.coreeventid.querymodelcompiling?view=efcore-3.1"/>
         /// </summary>
         internal const string EFQueryStartCompiling = "Microsoft.EntityFrameworkCore.Query.QueryCompilationStarting";
         /// <summary>
         /// Used for EF Core 2.X and 3.X.
-        /// <seealso href="https://docs.microsoft.com/dotnet/api/microsoft.entityframeworkcore.diagnostics.coreeventid.querymodelcompiling?view=efcore-3.1"></seealso>
+        /// <seealso href="https://docs.microsoft.com/dotnet/api/microsoft.entityframeworkcore.diagnostics.coreeventid.querymodelcompiling?view=efcore-3.1"/>
         /// </summary>
         internal const string EFQueryCompiling = "Microsoft.EntityFrameworkCore.Query.QueryModelCompiling";
         internal const string EFQueryCompiled = "Microsoft.EntityFrameworkCore.Query.QueryExecutionPlanned";
 
-        private IHub _hub { get; }
-        private SentryOptions _options { get; }
+        private readonly IHub _hub;
+        private readonly SentryOptions _options;
 
-        private AsyncLocal<WeakReference<ISpan>> _spansCompilerLocal = new();
-        private AsyncLocal<WeakReference<ISpan>> _spansQueryLocal = new();
-        private AsyncLocal<WeakReference<ISpan>> _spansConnectionLocal = new();
+        private readonly AsyncLocal<WeakReference<ISpan>> _spansCompilerLocal = new();
+        private readonly AsyncLocal<WeakReference<ISpan>> _spansQueryLocal = new();
+        private readonly AsyncLocal<WeakReference<ISpan>> _spansConnectionLocal = new();
 
         private bool _logConnectionEnabled = true;
         private bool _logQueryEnabled = true;
@@ -55,14 +53,20 @@ public SentryEFCoreListener(IHub hub, SentryOptions options)
 
         internal void DisableConnectionSpan() => _logConnectionEnabled = false;
 
-        internal bool DisableQuerySpan() => _logQueryEnabled = false;
+        internal void DisableQuerySpan() => _logQueryEnabled = false;
 
-        private ISpan? GetParent(SentryEFSpanType type, Scope scope)
-            => type == SentryEFSpanType.QueryExecution ? scope.GetSpan() : scope.Transaction;
+        private static ISpan? GetParent(SentryEFSpanType type, Scope scope)
+        {
+            if (type == SentryEFSpanType.QueryExecution)
+            {
+                return scope.GetSpan();
+            }
+
+            return scope.Transaction;
+        }
 
-        private ISpan? AddSpan(SentryEFSpanType type, string operation, string? description)
+        private void AddSpan(SentryEFSpanType type, string operation, string? description)
         {
-            ISpan? span = null;
             _hub.ConfigureScope(scope =>
             {
                 if (scope.Transaction?.IsSampled != true)
@@ -75,45 +79,40 @@ public SentryEFCoreListener(IHub hub, SentryOptions options)
                     return;
                 }
 
-                if (GetSpanBucket(type) is not { } asyncLocalSpan)
-                {
-                    return;
-                }
-
+                var asyncLocalSpan = GetSpanBucket(type);
                 asyncLocalSpan.Value = new WeakReference<ISpan>(startedChild);
-                span = startedChild;
             });
-            return span;
         }
 
         private ISpan? TakeSpan(SentryEFSpanType type)
         {
             ISpan? span = null;
             _hub.ConfigureScope(scope =>
             {
-                if (scope.Transaction?.IsSampled == true)
+                if (scope.Transaction?.IsSampled != true)
                 {
-                    if (GetSpanBucket(type)?.Value is { } reference &&
-                        reference.TryGetTarget(out var startedSpan))
-                    {
-                        span = startedSpan;
-                    }
-                    else
-                    {
-                        _options.LogWarning("Trying to close a span that was already garbage collected. {0}", type);
-                    }
+                    return;
+                }
+
+                if (GetSpanBucket(type).Value is { } reference &&
+                    reference.TryGetTarget(out var startedSpan))
+                {
+                    span = startedSpan;
+                    return;
                 }
+
+                _options.LogWarning("Trying to close a span that was already garbage collected. {0}", type);
             });
             return span;
         }
 
-        private AsyncLocal<WeakReference<ISpan>>? GetSpanBucket(SentryEFSpanType type)
+        private AsyncLocal<WeakReference<ISpan>> GetSpanBucket(SentryEFSpanType type)
             => type switch
             {
                 SentryEFSpanType.QueryCompiler => _spansCompilerLocal,
                 SentryEFSpanType.QueryExecution => _spansQueryLocal,
                 SentryEFSpanType.Connection => _spansConnectionLocal,
-                _ => null
+                _ => throw new ($"Unknown SentryEFSpanType: {type}")
             };
 
         public void OnCompleted() { }
@@ -125,41 +124,54 @@ public void OnNext(KeyValuePair<string, object?> value)
             try
             {
                 //Query compiler Span
-                if (value.Key == EFQueryStartCompiling || value.Key == EFQueryCompiling)
+                if (value.Key is EFQueryStartCompiling or EFQueryCompiling)
                 {
                     AddSpan(SentryEFSpanType.QueryCompiler, "db.query.compile", FilterNewLineValue(value.Value));
+                    return;
                 }
-                else if (value.Key == EFQueryCompiled)
+
+                if (value.Key == EFQueryCompiled)
                 {
                     TakeSpan(SentryEFSpanType.QueryCompiler)?.Finish(SpanStatus.Ok);
+                    return;
                 }
 
                 //Connection Span
                 //A transaction may or may not show a connection with it.
-                else if (_logConnectionEnabled && value.Key == EFConnectionOpening)
+
+                if (_logConnectionEnabled && value.Key == EFConnectionOpening)
                 {
                     AddSpan(SentryEFSpanType.Connection, "db.connection", null);
+                    return;
                 }
-                else if (_logConnectionEnabled && value.Key == EFConnectionClosed)
+
+                if (_logConnectionEnabled && value.Key == EFConnectionClosed)
                 {
                     TakeSpan(SentryEFSpanType.Connection)?.Finish(SpanStatus.Ok);
+                    return;
+                }
+
+                // return if not Query Execution Span
+                if (!_logQueryEnabled)
+                {
+                    return;
+                }
+
+                if (value.Key == EFCommandExecuting)
+                {
+                    AddSpan(SentryEFSpanType.QueryExecution, "db.query", FilterNewLineValue(value.Value));
+                    return;
+                }
+
+                if (value.Key == EFCommandFailed)
+                {
+                    TakeSpan(SentryEFSpanType.QueryExecution)?.Finish(SpanStatus.InternalError);
+                    return;
                 }
 
-                //Query Execution Span
-                else if (_logQueryEnabled)
+                if (value.Key == EFCommandExecuted)
                 {
-                    if (value.Key == EFCommandExecuting)
-                    {
-                        AddSpan(SentryEFSpanType.QueryExecution, "db.query", FilterNewLineValue(value.Value));
-                    }
-                    else if (value.Key == EFCommandFailed)
-                    {
-                        TakeSpan(SentryEFSpanType.QueryExecution)?.Finish(SpanStatus.InternalError);
-                    }
-                    else if (value.Key == EFCommandExecuted)
-                    {
-                        TakeSpan(SentryEFSpanType.QueryExecution)?.Finish(SpanStatus.Ok);
-                    }
+                    TakeSpan(SentryEFSpanType.QueryExecution)?.Finish(SpanStatus.Ok);
                 }
             }
             catch (Exception ex)