Support recoverability for Azure ServiceBus dead letter messages

src/Directory.Packages.props

@@ -47,7 +47,7 @@
     <PackageVersion Include="NServiceBus.RabbitMQ" Version="11.2.1" />
     <PackageVersion Include="NServiceBus.SagaAudit" Version="6.0.0" />
     <PackageVersion Include="NServiceBus.Testing" Version="10.0.2" />
-    <PackageVersion Include="NServiceBus.Transport.AzureServiceBus" Version="6.2.1" />
+    <PackageVersion Include="NServiceBus.Transport.AzureServiceBus" Version="6.3.0" />
     <PackageVersion Include="NServiceBus.Transport.AzureStorageQueues" Version="14.0.1" />
     <PackageVersion Include="NServiceBus.Transport.Msmq.Sources" Version="4.0.0" />
     <PackageVersion Include="NServiceBus.Transport.PostgreSql" Version="9.0.1" />

src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_ingesting_failed_message_with_missing_headers.cs

@@ -34,8 +34,8 @@ public async Task Should_be_ingested_when_minimal_required_headers_is_present()
         //No failure time will result in utc now being used
         Assert.That(failure.TimeOfFailure, Is.GreaterThan(testStartTime));
 
-        // Both host and endpoint name is currently needed so this will be null since no host can be detected from the failed q header
-        Assert.That(failure.ReceivingEndpoint, Is.Null);
+        Assert.That(failure.ReceivingEndpoint, Is.Not.Null);
+        Assert.That(failure.ReceivingEndpoint.Name, Is.EqualTo(context.EndpointNameOfReceivingEndpoint));
     }
 
     [Test]
@@ -45,10 +45,6 @@ public async Task Should_include_headers_required_by_ServicePulse()
             {
                 c.AddMinimalRequiredHeaders();
 
-                // This is needed for ServiceControl to be able to detect both endpoint (via failed q header) and host via the processing machine header
-                // Missing endpoint or host will cause a null ref in ServicePulse
-                c.Headers[Headers.ProcessingMachine] = "MyMachine";
-
                 c.Headers[FaultsHeaderKeys.ExceptionType] = "SomeExceptionType";
                 c.Headers[FaultsHeaderKeys.Message] = "Some message";
             })
@@ -62,8 +58,6 @@ public async Task Should_include_headers_required_by_ServicePulse()
 
         // ServicePulse assumes that the receiving endpoint name is present
         Assert.That(failure.ReceivingEndpoint, Is.Not.Null);
-        Assert.That(failure.ReceivingEndpoint.Name, Is.EqualTo(context.EndpointNameOfReceivingEndpoint));
-        Assert.That(failure.ReceivingEndpoint.Host, Is.EqualTo("MyMachine"));
 
         // ServicePulse needs both an exception type and description to render the UI in a resonable way
         Assert.That(failure.Exception.ExceptionType, Is.EqualTo("SomeExceptionType"));

src/ServiceControl.Transports/TransportCustomization.cs

@@ -138,15 +138,24 @@ public virtual async Task ProvisionQueues(TransportSettings transportSettings, I
                 true,
                 null); //null means "not hosted by core", transport SHOULD adjust accordingly to not assume things
 
-            var receivers = new[]{
+            var receiveQueueName = transportSettings.EndpointName;
+            var receivers = new[]
+            {
                 new ReceiveSettings(
                     transportSettings.EndpointName,
-                    new QueueAddress(transportSettings.EndpointName),
+                    new QueueAddress(receiveQueueName),
                     false,
                     false,
-                    transportSettings.ErrorQueue)};
+                    transportSettings.ErrorQueue)
+            };
+
+            var additionalQueuesToProvision = additionalQueues.Where(queueName => queueName != receiveQueueName)
+                .Union([transportSettings.ErrorQueue])
+                .Select(ToTransportQualifiedQueueNameCore)
+                .Distinct()
+                .ToArray();
 
-            var transportInfrastructure = await transport.Initialize(hostSettings, receivers, additionalQueues.Union([transportSettings.ErrorQueue]).Select(ToTransportQualifiedQueueNameCore).ToArray());
+            var transportInfrastructure = await transport.Initialize(hostSettings, receivers, additionalQueuesToProvision);
             await transportInfrastructure.Shutdown();
         }
 

src/ServiceControl.UnitTests/Operations/When_parsing_receive_endpoint.cs

@@ -0,0 +1,49 @@
+namespace ServiceControl.UnitTests.Operations;
+
+using System.Collections.Generic;
+using NServiceBus.Faults;
+using NUnit.Framework;
+using ServiceControl.Contracts.Operations;
+using ServiceControl.Infrastructure;
+
+[TestFixture]
+public class When_parsing_receive_endpoint
+{
+    [Test]
+    public void Should_infer_host_from_machine_name_in_failed_queue_when_host_header_is_missing()
+    {
+        var headers = new Dictionary<string, string>
+        {
+            { FaultsHeaderKeys.FailedQ, "Sales@backend-01" }
+        };
+
+        var endpoint = EndpointDetailsParser.ReceivingEndpoint(headers);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(endpoint, Is.Not.Null);
+            Assert.That(endpoint.Name, Is.EqualTo("Sales"));
+            Assert.That(endpoint.Host, Is.EqualTo("backend-01"));
+            Assert.That(endpoint.HostId, Is.EqualTo(DeterministicGuid.MakeId("Sales", "backend-01")));
+        });
+    }
+
+    [Test]
+    public void Should_fallback_to_unknown_if_host_can_not_be_determined()
+    {
+        var headers = new Dictionary<string, string>
+        {
+            { FaultsHeaderKeys.FailedQ, "Billing" }
+        };
+
+        var endpoint = EndpointDetailsParser.ReceivingEndpoint(headers);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(endpoint, Is.Not.Null);
+            Assert.That(endpoint.Name, Is.EqualTo("Billing"));
+            Assert.That(endpoint.Host, Is.EqualTo("unknown"));
+            Assert.That(endpoint.HostId, Is.EqualTo(DeterministicGuid.MakeId("Billing", "unknown")));
+        });
+    }
+}

src/ServiceControl.UnitTests/Recoverability/ExceptionTypeAndStackTraceFailureClassifierTest.cs

@@ -7,6 +7,8 @@
     [TestFixture]
     public class ExceptionTypeAndStackTraceFailureClassifierTest
     {
+        const string noStackTraceClassification = "exceptionType: No stacktrace";
+
         [Test]
         public void Failure_Without_ExceptionDetails_should_not_group()
         {
@@ -23,7 +25,7 @@ public void Empty_stack_trace_should_group_by_exception_type()
             var failureWithEmptyStackTrace = CreateFailureDetailsWithStackTrace(string.Empty);
             var classification = classifier.ClassifyFailure(failureWithEmptyStackTrace);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -33,7 +35,7 @@ public void Null_stack_trace_should_group_by_exception_type()
             var failureWithNullStackTrace = CreateFailureDetailsWithStackTrace(null);
             var classification = classifier.ClassifyFailure(failureWithNullStackTrace);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -43,7 +45,7 @@ public void Non_standard_stack_trace_format_should_group_by_exception_type()
             var failureWithNonStandardStackTrace = CreateFailureDetailsWithStackTrace("something other than a normal stack trace");
             var classification = classifier.ClassifyFailure(failureWithNonStandardStackTrace);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -53,7 +55,7 @@ public void Null_message_should_group_by_exception_type()
             var failureWithNullMessage = CreateFailureDetailsWithMessage(null);
             var classification = classifier.ClassifyFailure(failureWithNullMessage);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -63,7 +65,7 @@ public void Empty_message_should_group_by_exception_type()
             var failureWithEmptyMessage = CreateFailureDetailsWithMessage(string.Empty);
             var classification = classifier.ClassifyFailure(failureWithEmptyMessage);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -73,7 +75,7 @@ public void Whitespace_message_should_group_by_exception_type()
             var failureWithWhitespaceMessage = CreateFailureDetailsWithMessage("   ");
             var classification = classifier.ClassifyFailure(failureWithWhitespaceMessage);
 
-            Assert.That(classification, Is.EqualTo("exceptionType: 0"));
+            Assert.That(classification, Is.EqualTo(noStackTraceClassification));
         }
 
         [Test]
@@ -154,4 +156,4 @@ static ClassifiableMessageDetails CreateFailureDetailsWithMessage(string message
             return new ClassifiableMessageDetails(null, failure, null);
         }
     }
-}
+}

src/ServiceControl/Operations/EndpointDetailsParser.cs

@@ -77,12 +77,17 @@ public static EndpointDetails ReceivingEndpoint(IReadOnlyDictionary<string, stri
 
                 if (string.IsNullOrEmpty(endpoint.Host))
                 {
-                    endpoint.Host = queueAndMachinename.Machine;
+                    endpoint.Host = queueAndMachinename.Machine ?? "unknown";
                 }
 
                 // If we've been now able to get the endpoint details, return the new info.
-                if (!string.IsNullOrEmpty(endpoint.Name) && !string.IsNullOrEmpty(endpoint.Host))
+                if (!string.IsNullOrEmpty(endpoint.Name))
                 {
+                    if (endpoint.HostId == Guid.Empty)
+                    {
+                        endpoint.HostId = DeterministicGuid.MakeId(endpoint.Name, endpoint.Host);
+                    }
+
                     return endpoint;
                 }
             }

src/ServiceControl/Recoverability/Grouping/Groupers/ExceptionTypeAndStackTraceFailureClassifier.cs

@@ -29,6 +29,7 @@ public string ClassifyFailure(ClassifiableMessageDetails failure)
                 // We need to remove the message in order to make sure the stack trace parser does not get into catastrophic backtracking mode.
                 exceptionStackTrace = exceptionStackTrace.Replace(exception.Message, string.Empty);
             }
+
             try
             {
                 var firstStackTraceFrame = StackTraceParser.Parse(exceptionStackTrace, (frame, type, method, parameterList, parameters, file, line) => new StackFrame
@@ -54,10 +55,7 @@ public string ClassifyFailure(ClassifiableMessageDetails failure)
             return GetNonStandardClassification(exception.ExceptionType);
         }
 
-        static string GetNonStandardClassification(string exceptionType)
-        {
-            return exceptionType + ": 0";
-        }
+        static string GetNonStandardClassification(string exceptionType) => exceptionType + ": No stacktrace";
 
         public const string Id = "Exception Type and Stack Trace";