Tracing now works for SentryHttpMessageHandler even when there is no active transaction

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+- Fixed SentryHttpMessageHandler and SentryGraphQLHttpMessageHandler not creating spans when there is no active Transaction on the scope ([#3360](https://github.com/getsentry/sentry-dotnet/pull/3360))
+
 ## 4.6.0
 
 ### Features

src/Sentry.OpenTelemetry/SentrySpanProcessor.cs

@@ -40,13 +40,7 @@ public SentrySpanProcessor(IHub hub) : this(hub, null)
     internal SentrySpanProcessor(IHub hub, IEnumerable<IOpenTelemetryEnricher>? enrichers)
     {
         _hub = hub;
-        _realHub = new Lazy<Hub?>(() =>
-            _hub switch
-            {
-                Hub thisHub => thisHub,
-                HubAdapter when SentrySdk.CurrentHub is Hub sdkHub => sdkHub,
-                _ => null
-            });
+        _realHub = new Lazy<Hub?>(() => _hub.GetRealHub());
 
         if (_hub is DisabledHub)
         {

src/Sentry/HubExtensions.cs

@@ -244,4 +244,11 @@ internal static ITransactionTracer StartTransaction(
         var transaction = hub.GetTransaction();
         return transaction?.IsSampled == true ? transaction : null;
     }
+
+    internal static Hub? GetRealHub(this IHub hub) => hub switch
+    {
+        Hub thisHub => thisHub,
+        HubAdapter when SentrySdk.CurrentHub is Hub sdkHub => sdkHub,
+        _ => null
+    };
 }

src/Sentry/IMetricHub.cs

@@ -17,8 +17,14 @@ internal interface IMetricHub
     /// <summary>
     /// Starts a child span for the current transaction or, if there is no active transaction, starts a new transaction.
     /// </summary>
-    ISpan StartSpan(string operation, string description);
+    ISpan StartSpan(string name, string operation, string description);
 
     /// <inheritdoc cref="IHub.GetSpan"/>
     ISpan? GetSpan();
 }
+
+internal static class MetricHubExtensions
+{
+    public static ISpan StartSpan(this IMetricHub hub, string operation, string description) =>
+        hub.StartSpan(description, operation, description);
+}

src/Sentry/Internal/Hub.cs

@@ -531,13 +531,13 @@ public void CaptureCodeLocations(CodeLocations codeLocations)
     }
 
     /// <inheritdoc cref="IMetricHub.StartSpan"/>
-    public ISpan StartSpan(string operation, string description)
+    public ISpan StartSpan(string name, string operation, string description)
     {
         ITransactionTracer? currentTransaction = null;
         ConfigureScope(s => currentTransaction = s.Transaction);
         return currentTransaction is { } transaction
             ? transaction.StartChild(operation, description)
-            : this.StartTransaction(operation, description);
+            : this.StartTransaction(name, operation, description);
     }
 
     public void CaptureSession(SessionUpdate sessionUpdate)

src/Sentry/SentryGraphQLHttpMessageHandler.cs

@@ -12,6 +12,7 @@ public class SentryGraphQLHttpMessageHandler : SentryMessageHandler
     private readonly IHub _hub;
     private readonly SentryOptions? _options;
     private readonly ISentryFailedRequestHandler? _failedRequestHandler;
+    private readonly Lazy<Hub?> _realHub;
 
     /// <summary>
     /// Constructs an instance of <see cref="SentryGraphQLHttpMessageHandler"/>.
@@ -29,6 +30,7 @@ internal SentryGraphQLHttpMessageHandler(IHub? hub, SentryOptions? options,
     : base(hub, options, innerHandler)
     {
         _hub = hub ?? HubAdapter.Instance;
+        _realHub = new Lazy<Hub?>(() => _hub.GetRealHub());
         _options = options ?? _hub.GetSentryOptions();
         _failedRequestHandler = failedRequestHandler;
         if (_options != null)
@@ -50,7 +52,7 @@ internal SentryGraphQLHttpMessageHandler(IHub? hub, SentryOptions? options,
 
         // Start a span that tracks this request
         // (may be null if transaction is not set on the scope)
-        var span = _hub.GetSpan()?.StartChild(
+        var span = _realHub.Value?.StartSpan(
             "http.client",
             $"{method} {url}" // e.g. "GET https://example.com"
         );

src/Sentry/SentryHttpMessageHandler.cs

@@ -1,4 +1,5 @@
 using Sentry.Extensibility;
+using Sentry.Internal;
 using Sentry.Internal.OpenTelemetry;
 
 namespace Sentry;
@@ -11,6 +12,7 @@ public class SentryHttpMessageHandler : SentryMessageHandler
     private readonly IHub _hub;
     private readonly SentryOptions? _options;
     private readonly ISentryFailedRequestHandler? _failedRequestHandler;
+    private readonly Lazy<Hub?> _realHub;
 
     /// <summary>
     /// Constructs an instance of <see cref="SentryHttpMessageHandler"/>.
@@ -48,6 +50,7 @@ internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessage
         : base(hub, options, innerHandler)
     {
         _hub = hub ?? HubAdapter.Instance;
+        _realHub = new Lazy<Hub?>(() => _hub.GetRealHub());
         _options = options ?? _hub.GetSentryOptions();
         _failedRequestHandler = failedRequestHandler;
 
@@ -63,7 +66,7 @@ internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessage
     {
         // Start a span that tracks this request
         // (may be null if transaction is not set on the scope)
-        var span = _hub.GetSpan()?.StartChild(
+        var span = _realHub.Value?.StartSpan(
             "http.client",
             $"{method} {url}" // e.g. "GET https://example.com"
             );

test/Sentry.Tests/SentryGraphQlHttpMessageHandlerTests.cs

@@ -7,7 +7,7 @@ namespace Sentry.Tests;
  * TODO: Find a way to consolidate these tests cleanly.
  */
 
-public class SentryGraphQlHttpMessageHandlerTests
+public class SentryGraphQlHttpMessageHandlerTests : SentryMessageHandlerTests
 {
     private const string ValidQuery = "query getAllNotes { notes { id } }";
     private const string ValidResponse = @"{
@@ -47,13 +47,7 @@ public void ProcessRequest_ExtractsGraphQlRequestContent()
     public void ProcessRequest_SetsSpanData()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
-        var parentSpan = Substitute.For<ISpan>();
-        hub.GetSpan().Returns(parentSpan);
-        var childSpan = Substitute.For<ISpan>();
-        parentSpan.When(p => p.StartChild(Arg.Any<string>()))
-                  .Do(op => childSpan.Operation = op.Arg<string>());
-        parentSpan.StartChild(Arg.Any<string>()).Returns(childSpan);
+        var hub = _fixture.GetHub();
         var sut = new SentryGraphQLHttpMessageHandler(hub, null);
 
         var method = "POST";
@@ -66,9 +60,15 @@ public void ProcessRequest_SetsSpanData()
 
         // Assert
         returnedSpan.Should().NotBeNull();
-        returnedSpan!.Operation.Should().Be("http.client");
-        returnedSpan.Description.Should().Be($"{method} {url}");
-        returnedSpan.Received(1).SetExtra(OtelSemanticConventions.AttributeHttpRequestMethod, method);
+        using (new AssertionScope())
+        {
+            returnedSpan!.Operation.Should().Be("http.client");
+            returnedSpan.Description.Should().Be($"{method} {url}");
+            returnedSpan.Extra.Should().Contain(kvp =>
+                kvp.Key == OtelSemanticConventions.AttributeHttpRequestMethod &&
+                kvp.Value.ToString() == method
+            );
+        }
     }
 
     // [Theory]
@@ -118,14 +118,13 @@ public void HandleResponse_AddsBreadcrumb()
     public void HandleResponse_SetsSpanData()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
+        var hub = _fixture.GetHub();
         var status = HttpStatusCode.OK;
         var response = new HttpResponseMessage(status);
         var method = "POST";
         var url = "http://example.com/graphql";
         var request = SentryGraphQlTestHelpers.GetRequestQuery(ValidQuery, url);
         response.RequestMessage = request;
-        hub.GetSpan().Returns(new TransactionTracer(hub, "test", "test"));
         var sut = new SentryGraphQLHttpMessageHandler(hub, null);
 
         // Act

test/Sentry.Tests/SentryHttpMessageHandlerTests.cs

@@ -7,7 +7,7 @@ namespace Sentry.Tests;
  * TODO: Find a way to consolidate these tests cleanly.
  */
 
-public class SentryHttpMessageHandlerTests
+public class SentryHttpMessageHandlerTests : SentryMessageHandlerTests
 {
     [Fact]
     public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader_ByDefault()
@@ -122,17 +122,12 @@ public async Task SendAsync_SentryTraceHeaderAlreadySet_NotOverwritten()
     }
 
     [Fact]
-    public async Task SendAsync_TransactionOnScope_StartsNewSpan()
+    public async Task SendAsync_TransactionOnScope_StartsChildSpan()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
-
-        var transaction = new TransactionTracer(
-            hub,
-            "foo",
-            "bar");
-
-        hub.GetSpan().ReturnsForAnyArgs(transaction);
+        var hub = _fixture.GetHub();
+        var transaction = hub.StartTransaction("foo", "bar");
+        hub.ConfigureScope(scope => scope.Transaction = transaction);
 
         using var innerHandler = new FakeHttpMessageHandler();
         using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub);
@@ -149,17 +144,40 @@ public async Task SendAsync_TransactionOnScope_StartsNewSpan()
     }
 
     [Fact]
-    public async Task SendAsync_ExceptionThrown_ExceptionLinkedToSpan()
+    public async Task SendAsync_NoTransactionOnScope_StartsTransaction()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
+        SentryTransaction received = null;
+        _fixture.Client.CaptureTransaction(
+            Arg.Do<SentryTransaction>(t => received = t),
+            Arg.Any<Scope>(),
+            Arg.Any<SentryHint>()
+        );
+        var hub = _fixture.GetHub();
 
-        var transaction = new TransactionTracer(
-            hub,
-            "foo",
-            "bar");
+        using var innerHandler = new FakeHttpMessageHandler();
+        using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub);
+        using var client = new HttpClient(sentryHandler);
+
+        // Act
+        await client.GetAsync("https://localhost/");
+
+        // Assert
+        received.Should().NotBeNull();
+        using (new AssertionScope())
+        {
+            received.Name.Should().Be("GET https://localhost/");
+            received.Operation.Should().Be("http.client");
+            received.Description.Should().Be("GET https://localhost/");
+            received.IsFinished.Should().BeTrue();
+        }
+    }
 
-        hub.GetSpan().ReturnsForAnyArgs(transaction);
+    [Fact]
+    public async Task SendAsync_ExceptionThrown_ExceptionLinkedToSpan()
+    {
+        // Arrange
+        var hub = _fixture.GetHub();
 
         var exception = new Exception();
 
@@ -171,7 +189,8 @@ public async Task SendAsync_ExceptionThrown_ExceptionLinkedToSpan()
         await Assert.ThrowsAsync<Exception>(() => client.GetAsync("https://localhost/"));
 
         // Assert
-        hub.Received(1).BindException(exception, Arg.Any<ISpan>()); // second argument is an implicitly created span
+        hub.ExceptionToSpanMap.TryGetValue(exception, out var span).Should().BeTrue();
+        span.Should().NotBeNull();
     }
 
     [Fact]
@@ -243,17 +262,12 @@ public async Task SendAsync_Executed_FailedRequestsCaptured()
     public void ProcessRequest_SetsSpanData()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
-        var parentSpan = Substitute.For<ISpan>();
-        hub.GetSpan().Returns(parentSpan);
-        var childSpan = Substitute.For<ISpan>();
-        parentSpan.When(p => p.StartChild(Arg.Any<string>()))
-            .Do(op => childSpan.Operation = op.Arg<string>());
-        parentSpan.StartChild(Arg.Any<string>()).Returns(childSpan);
-        var sut = new SentryHttpMessageHandler(hub, null);
-
-        var method = "GET";
-        var url = "http://example.com/graphql";
+        var hub = _fixture.GetHub();
+        using var innerHandler = new FakeHttpMessageHandler();
+        var sut = new SentryHttpMessageHandler(hub, _fixture.Options, innerHandler);
+
+        const string method = "GET";
+        const string url = "https://example.com/graphql";
         var request = new HttpRequestMessage(HttpMethod.Get, url);
 
         // Act
@@ -263,7 +277,10 @@ public void ProcessRequest_SetsSpanData()
         returnedSpan.Should().NotBeNull();
         returnedSpan!.Operation.Should().Be("http.client");
         returnedSpan.Description.Should().Be($"{method} {url}");
-        returnedSpan.Received(1).SetExtra(OtelSemanticConventions.AttributeHttpRequestMethod, method);
+        returnedSpan.Extra.Should().Contain(kvp =>
+            kvp.Key == OtelSemanticConventions.AttributeHttpRequestMethod &&
+            Equals(kvp.Value, method)
+        );
     }
 
     [Fact]
@@ -407,14 +424,9 @@ public void Send_SentryTraceHeaderAlreadySet_NotOverwritten()
     public void Send_TransactionOnScope_StartsNewSpan()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
-
-        var transaction = new TransactionTracer(
-            hub,
-            "foo",
-            "bar");
-
-        hub.GetSpan().ReturnsForAnyArgs(transaction);
+        var hub = _fixture.GetHub();
+        var transaction = hub.StartTransaction("foo", "bar");
+        hub.ConfigureScope(scope => scope.Transaction = transaction);
 
         using var innerHandler = new FakeHttpMessageHandler();
         using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub);
@@ -434,14 +446,7 @@ public void Send_TransactionOnScope_StartsNewSpan()
     public void Send_ExceptionThrown_ExceptionLinkedToSpan()
     {
         // Arrange
-        var hub = Substitute.For<IHub>();
-
-        var transaction = new TransactionTracer(
-            hub,
-            "foo",
-            "bar");
-
-        hub.GetSpan().ReturnsForAnyArgs(transaction);
+        var hub = _fixture.GetHub();
 
         var exception = new Exception();
 
@@ -453,7 +458,8 @@ public void Send_ExceptionThrown_ExceptionLinkedToSpan()
         Assert.Throws<Exception>(() => client.Get("https://localhost/"));
 
         // Assert
-        hub.Received(1).BindException(exception, Arg.Any<ISpan>()); // second argument is an implicitly created span
+        hub.ExceptionToSpanMap.TryGetValue(exception, out var span).Should().BeTrue();
+        span.Should().NotBeNull();
     }
 
     [Fact]

test/Sentry.Tests/SentryMessageHandlerTests.cs

@@ -0,0 +1,31 @@
+using Sentry.Internal.OpenTelemetry;
+
+namespace Sentry.Tests;
+
+public class SentryMessageHandlerTests
+{
+    private protected class Fixture
+    {
+        public readonly SentryOptions Options = new();
+
+        public readonly ISentryClient Client = Substitute.For<ISentryClient>();
+
+        public readonly ISessionManager SessionManager = Substitute.For<ISessionManager>();
+
+        public readonly ISystemClock Clock = Substitute.For<ISystemClock>();
+
+        public Fixture()
+        {
+            Options.Dsn = ValidDsn;
+            Options.TracesSampleRate = 1.0;
+        }
+
+        public Hub GetHub()
+        {
+            var scopeManager = new SentryScopeManager(Options, Client);
+            return new Hub(Options, Client, SessionManager, Clock, scopeManager);
+        }
+    }
+
+    private protected readonly Fixture _fixture = new();
+}