Fix DiagnosticSource integration disabled incorrectly with TracesSamp…

…ler (#2039)
getsentry · Nov 10, 2022 · b8e01bf · b8e01bf
1 parent 6af8ce3
commit b8e01bf
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 - Update unobserved task exception integration ([#2034](https://github.com/getsentry/sentry-dotnet/pull/2034))
 - Fix trace propagation targets setter ([#2035](https://github.com/getsentry/sentry-dotnet/pull/2035))
+- Fix DiagnosticSource integration disabled incorrectly with TracesSampler ([#2039](https://github.com/getsentry/sentry-dotnet/pull/2039))
 
 ## 3.23.1
 

diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs
@@ -6,19 +6,16 @@ namespace Sentry.Internal.DiagnosticSource;
 
 internal class SentryDiagnosticListenerIntegration : ISdkIntegration
 {
-    private SentryDiagnosticSubscriber? _subscriber;
-    private IDisposable? _diagnosticListener;
-
     public void Register(IHub hub, SentryOptions options)
     {
-        if (options.TracesSampleRate == 0)
+        if (options.TracesSampleRate == 0 && options.TracesSampler == null)
         {
-            options.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, and no TracesSampler set.");
             options.DisableDiagnosticSourceIntegration();
             return;
         }
 
-        _subscriber = new SentryDiagnosticSubscriber(hub, options);
-        _diagnosticListener = DiagnosticListener.AllListeners.Subscribe(_subscriber);
+        var subscriber = new SentryDiagnosticSubscriber(hub, options);
+        DiagnosticListener.AllListeners.Subscribe(subscriber);
     }
 }
diff --git a/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs b/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs
@@ -4,33 +4,80 @@ namespace Sentry.DiagnosticSource.Tests;
 
 public class DiagnosticsSentryOptionsExtensionsTests
 {
+    private readonly SentryOptions _options = new()
+    {
+        Dsn = ValidDsn,
+        AutoSessionTracking = false,
+        IsGlobalModeEnabled = true,
+        BackgroundWorker = Substitute.For<IBackgroundWorker>(),
+
+        // Set explicitly for this test in case the defaults change in the future.
+        TracesSampleRate = 0.0,
+        TracesSampler = null
+    };
+
+    public DiagnosticsSentryOptionsExtensionsTests(ITestOutputHelper output)
+    {
+#if NETFRAMEWORK
+        _options.AddDiagnosticSourceIntegration();
+#endif
+    }
+
+    private Hub GetSut() => new(_options, Substitute.For<ISentryClient>());
+
+    private static IEnumerable<ISdkIntegration> GetIntegrations(ISentryClient hub) =>
+        hub.GetSentryOptions()?.Integrations ?? Enumerable.Empty<ISdkIntegration>();
+
     [Fact]
-    public void DisableDiagnosticListenerIntegration_RemovesDiagnosticSourceIntegration()
+    public void DiagnosticListenerIntegration_DisabledWithoutTracesSampling()
     {
-        var options = new SentryOptions();
-        options.DisableDiagnosticSourceIntegration();
-        Assert.DoesNotContain(options.Integrations!,
-            p => p is SentryDiagnosticListenerIntegration);
+        using var hub = GetSut();
+        var integrations = GetIntegrations(hub);
+        Assert.DoesNotContain(integrations, _ => _ is SentryDiagnosticListenerIntegration);
     }
 
-#if NETFRAMEWORK
     [Fact]
-    public void AddDiagnosticSourceIntegration()
+    public void DiagnosticListenerIntegration_EnabledWithTracesSampleRate()
     {
-        var options = new SentryOptions();
-        options.AddDiagnosticSourceIntegration();
-        Assert.Contains(options.Integrations!,
-            p => p is SentryDiagnosticListenerIntegration);
+        _options.TracesSampleRate = 1.0;
+
+        using var hub = GetSut();
+        var integrations = GetIntegrations(hub);
+        Assert.Contains(integrations, _ => _ is SentryDiagnosticListenerIntegration);
+    }
+
+    [Fact]
+    public void DiagnosticListenerIntegration_EnabledWithTracesSampler()
+    {
+        // It doesn't matter what the sampler returns, just that it exists.
+        _options.TracesSampler = _ => null;
+
+        using var hub = GetSut();
+        var integrations = GetIntegrations(hub);
+        Assert.Contains(integrations, _ => _ is SentryDiagnosticListenerIntegration);
     }
 
     [Fact]
-    public void AddDiagnosticSourceIntegration_NoDuplicates()
+    public void DisableDiagnosticListenerIntegration_RemovesDiagnosticSourceIntegration()
+    {
+        _options.TracesSampleRate = 1.0;
+        _options.DisableDiagnosticSourceIntegration();
+
+        using var hub = GetSut();
+        var integrations = GetIntegrations(hub);
+        Assert.DoesNotContain(integrations, _ => _ is SentryDiagnosticListenerIntegration);
+    }
+
+#if NETFRAMEWORK
+    [Fact]
+    public void DiagnosticListenerIntegration_AddDiagnosticSourceIntegration_DoesntDuplicate()
     {
         var options = new SentryOptions();
+
         options.AddDiagnosticSourceIntegration();
         options.AddDiagnosticSourceIntegration();
-        Assert.Single(options.Integrations!,
-            p => p is SentryDiagnosticListenerIntegration);
+
+        Assert.Single(options.Integrations!, _ => _ is SentryDiagnosticListenerIntegration);
     }
 #endif
 }