OpenTelemetry integration throws an exception when Sentry is disabled (#3156)

Author: jamescrosswell

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Metric unit names are now sanitized correctly. This was preventing some built in metrics from showing in the Sentry dashboard ([#3151](https://github.com/getsentry/sentry-dotnet/pull/3151))
+- The Sentry OpenTelemetry integration no longer throws an exception with the SDK disabled ([#3156](https://github.com/getsentry/sentry-dotnet/pull/3156))
 
 ## 4.1.1
 

src/Sentry.OpenTelemetry/DisabledSpanProcessor.cs

@@ -0,0 +1,19 @@
+using OpenTelemetry;
+
+namespace Sentry.OpenTelemetry;
+
+internal class DisabledSpanProcessor : BaseProcessor<Activity>
+{
+    private static readonly Lazy<DisabledSpanProcessor> LazyInstance = new();
+    internal static DisabledSpanProcessor Instance => LazyInstance.Value;
+
+    public override void OnStart(Activity activity)
+    {
+        // No-Op
+    }
+
+    public override void OnEnd(Activity activity)
+    {
+        // No-Op
+    }
+}

src/Sentry.OpenTelemetry/SentrySpanProcessor.cs

@@ -12,7 +12,7 @@ namespace Sentry.OpenTelemetry;
 public class SentrySpanProcessor : BaseProcessor<Activity>
 {
     private readonly IHub _hub;
-    private readonly IEnumerable<IOpenTelemetryEnricher> _enrichers;
+    internal readonly IEnumerable<IOpenTelemetryEnricher> _enrichers;
 
     // ReSharper disable once MemberCanBePrivate.Global - Used by tests
     internal readonly ConcurrentDictionary<ActivitySpanId, ISpan> _map = new();
@@ -36,14 +36,23 @@ public SentrySpanProcessor(IHub hub) : this(hub, null)
     internal SentrySpanProcessor(IHub hub, IEnumerable<IOpenTelemetryEnricher>? enrichers)
     {
         _hub = hub;
+
+        if (_hub is DisabledHub)
+        {
+            // This would only happen if someone tried to create a SentrySpanProcessor manually
+            throw new InvalidOperationException(
+                "Attempted to creates a SentrySpanProcessor for a Disabled hub. " +
+                "You should use the TracerProviderBuilderExtensions to configure Sentry with OpenTelemetry");
+        }
+
         _enrichers = enrichers ?? Enumerable.Empty<IOpenTelemetryEnricher>();
         _options = hub.GetSentryOptions();
 
-        if (_options is not { })
+        if (_options is null)
         {
             throw new InvalidOperationException(
-                "The Sentry SDK has not been initialised. To use Sentry with OpenTelemetry tracing you need to " +
-                "initialize the Sentry SDK.");
+                "The Sentry SDK has not been initialised. To use Sentry with OpenTelemetry " +
+                "tracing you need to initialize the Sentry SDK.");
         }
 
         if (_options.Instrumenter != Instrumenter.OpenTelemetry)

src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs

@@ -2,6 +2,7 @@
 using OpenTelemetry;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Trace;
+using Sentry.Extensibility;
 
 namespace Sentry.OpenTelemetry;
 
@@ -30,19 +31,28 @@ public static TracerProviderBuilder AddSentry(this TracerProviderBuilder tracerP
     {
         defaultTextMapPropagator ??= new SentryPropagator();
         Sdk.SetDefaultTextMapPropagator(defaultTextMapPropagator);
-        return tracerProviderBuilder.AddProcessor(services =>
-        {
-            List<IOpenTelemetryEnricher> enrichers = new();
+        return tracerProviderBuilder.AddProcessor(ImplementationFactory);
+    }
 
-            // AspNetCoreEnricher
-            var userFactory = services.GetService<ISentryUserFactory>();
-            if (userFactory is not null)
-            {
-                enrichers.Add(new AspNetCoreEnricher(userFactory));
-            }
+    internal static BaseProcessor<Activity> ImplementationFactory(IServiceProvider services)
+    {
+        List<IOpenTelemetryEnricher> enrichers = new();
 
-            var hub = services.GetService<IHub>() ?? SentrySdk.CurrentHub;
+        // AspNetCoreEnricher
+        var userFactory = services.GetService<ISentryUserFactory>();
+        if (userFactory is not null)
+        {
+            enrichers.Add(new AspNetCoreEnricher(userFactory));
+        }
+
+        var hub = services.GetService<IHub>() ?? SentrySdk.CurrentHub;
+        if (hub.IsEnabled)
+        {
             return new SentrySpanProcessor(hub, enrichers);
-        });
+        }
+
+        var logger = services.GetService<IDiagnosticLogger>();
+        logger?.LogWarning("Sentry is disabled so no OpenTelemetry spans will be sent to Sentry.");
+        return DisabledSpanProcessor.Instance;
     }
 }

test/Sentry.OpenTelemetry.Tests/TracerProviderBuilderExtensionsTests.cs

@@ -0,0 +1,94 @@
+namespace Sentry.OpenTelemetry.Tests;
+
+public class TracerProviderBuilderExtensionsTests
+{
+    private class Fixture
+    {
+        public IServiceProvider ServiceProvider { get; } = Substitute.For<IServiceProvider>();
+        public IHub Hub { get; } = Substitute.For<IHub>();
+
+        public Fixture()
+        {
+            ServiceProvider.GetService(typeof(IHub)).Returns(Hub);
+        }
+
+        public SentryOptions GetOptions(string dsn = "https://[email protected]/789") => new()
+        {
+            Instrumenter = Instrumenter.OpenTelemetry,
+            Dsn = dsn
+        };
+
+        public IServiceProvider GetServiceProvider() => ServiceProvider;
+    }
+
+
+    [Fact]
+    public void ImplementationFactory_WithUserFactory_AddsAspNetCoreEnricher()
+    {
+        // Arrange
+        var fixture = new Fixture();
+        SentryClientExtensions.SentryOptionsForTestingOnly = fixture.GetOptions();
+        fixture.Hub.IsEnabled.Returns(true);
+        var userFactory = Substitute.For<ISentryUserFactory>();
+        fixture.ServiceProvider.GetService(typeof(ISentryUserFactory)).Returns(userFactory);
+        var services = fixture.GetServiceProvider();
+
+        // Act
+        var result = TracerProviderBuilderExtensions.ImplementationFactory(services);
+
+        // Assert
+        result.Should().BeOfType<SentrySpanProcessor>(); // FluentAssertions
+        var spanProcessor = (SentrySpanProcessor)result;
+        spanProcessor._enrichers.Should().NotBeEmpty();
+    }
+
+    [Fact]
+    public void ImplementationFactory_WithoutUserFactory_DoesNotAddAspNetCoreEnricher()
+    {
+        // Arrange
+        var fixture = new Fixture();
+        SentryClientExtensions.SentryOptionsForTestingOnly = fixture.GetOptions();
+        fixture.Hub.IsEnabled.Returns(true);
+        var services = fixture.GetServiceProvider();
+
+        // Act
+        var result = TracerProviderBuilderExtensions.ImplementationFactory(services);
+
+        // Assert
+        result.Should().BeOfType<SentrySpanProcessor>(); // FluentAssertions
+        var spanProcessor = (SentrySpanProcessor)result;
+        spanProcessor._enrichers.Should().BeEmpty();
+    }
+
+    [Fact]
+    public void ImplementationFactory_WithEnabledHub_ReturnsSentrySpanProcessor()
+    {
+        // Arrange
+        var fixture = new Fixture();
+        SentryClientExtensions.SentryOptionsForTestingOnly = fixture.GetOptions();
+        fixture.Hub.IsEnabled.Returns(true);
+        var services = fixture.GetServiceProvider();
+
+        // Act
+        var result = TracerProviderBuilderExtensions.ImplementationFactory(services);
+
+        // Assert
+        result.Should().BeOfType<SentrySpanProcessor>(); // FluentAssertions
+    }
+
+    [Fact]
+    public void ImplementationFactory_WithDisabledHub_ReturnsDisabledSpanProcessor()
+    {
+        // Arrange
+        var fixture = new Fixture();
+        SentryClientExtensions.SentryOptionsForTestingOnly = fixture.GetOptions();
+        fixture.Hub.IsEnabled.Returns(false);
+        var services = fixture.GetServiceProvider();
+
+        // Act
+        var result = TracerProviderBuilderExtensions.ImplementationFactory(services);
+
+        // Assert
+        result.Should().BeOfType<DisabledSpanProcessor>(); // FluentAssertions
+    }
+}