Skip to content
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

Fix DiagnosticSource integration disabled incorrectly with TracesSampler #2039

Merged
merged 4 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}