From f9a64237f3bda9e39ff3a4be2d5e0ec1f1e7af0b Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Fri, 18 Jun 2021 15:17:17 +0200 Subject: [PATCH 01/42] Add version header --- .../Recoverability/Retrying/Infrastructure/ReturnToSender.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs index e470e22703..de0fe468c7 100644 --- a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs +++ b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs @@ -27,6 +27,7 @@ public virtual async Task HandleMessage(MessageContext message, IDispatchMessage var outgoingHeaders = new Dictionary(message.Headers); outgoingHeaders.Remove("ServiceControl.Retry.StagingId"); + outgoingHeaders["ServiceControl.Version"] = GitVersionInformation.MajorMinorPatch; byte[] body = null; var messageId = message.MessageId; From eeb90cc48cd3b5df59fe98262c4784dc423acf3d Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 21 Jun 2021 13:39:34 +0200 Subject: [PATCH 02/42] Move Persist error handling to the ErrorIngestor --- .../Operations/ErrorIngestionComponent.cs | 2 +- .../Operations/ErrorIngestor.cs | 53 ++++++++++++- .../Operations/ErrorPersister.cs | 79 ++++++------------- 3 files changed, 74 insertions(+), 60 deletions(-) diff --git a/src/ServiceControl/Operations/ErrorIngestionComponent.cs b/src/ServiceControl/Operations/ErrorIngestionComponent.cs index d92a047241..98edefa1a7 100644 --- a/src/ServiceControl/Operations/ErrorIngestionComponent.cs +++ b/src/ServiceControl/Operations/ErrorIngestionComponent.cs @@ -53,7 +53,7 @@ ErrorIngestionCustomCheck.State ingestionState this.settings = settings; var announcer = new FailedMessageAnnouncer(domainEvents); persister = new ErrorPersister(documentStore, bodyStorageEnricher, enrichers, failedMessageEnrichers, ingestedMeter, bulkInsertDurationMeter); - ingestor = new ErrorIngestor(persister, announcer, settings.ForwardErrorMessages, settings.ErrorLogQueue); + ingestor = new ErrorIngestor(persister, announcer, documentStore, bulkInsertDurationMeter, settings.ForwardErrorMessages, settings.ErrorLogQueue); var ingestion = new ErrorIngestion(async messageContext => { diff --git a/src/ServiceControl/Operations/ErrorIngestor.cs b/src/ServiceControl/Operations/ErrorIngestor.cs index edae423005..7500c14b71 100644 --- a/src/ServiceControl/Operations/ErrorIngestor.cs +++ b/src/ServiceControl/Operations/ErrorIngestor.cs @@ -2,26 +2,31 @@ { using System; using System.Collections.Generic; + using System.Diagnostics; using System.Threading.Tasks; using Contracts.Operations; + using Infrastructure.Metrics; using NServiceBus.Extensibility; using NServiceBus.Logging; using NServiceBus.Routing; using NServiceBus.Transport; + using Raven.Client; class ErrorIngestor { - public ErrorIngestor(ErrorPersister errorPersister, FailedMessageAnnouncer failedMessageAnnouncer, bool forwardErrorMessages, string errorLogQueue) + public ErrorIngestor(ErrorPersister errorPersister, FailedMessageAnnouncer failedMessageAnnouncer, IDocumentStore store, Meter bulkInsertDurationMeter, bool forwardErrorMessages, string errorLogQueue) { this.errorPersister = errorPersister; this.failedMessageAnnouncer = failedMessageAnnouncer; + this.store = store; + this.bulkInsertDurationMeter = bulkInsertDurationMeter; this.forwardErrorMessages = forwardErrorMessages; this.errorLogQueue = errorLogQueue; } public async Task Ingest(List contexts) { - var stored = await errorPersister.Persist(contexts) + var stored = await Persist(contexts) .ConfigureAwait(false); try @@ -64,6 +69,48 @@ public async Task Ingest(List contexts) } } + public async Task> Persist(List contexts) + { + var stopwatch = Stopwatch.StartNew(); + + if (log.IsDebugEnabled) + { + log.Debug($"Batch size {contexts.Count}"); + } + + try + { + var (storedContexts, commands) = await errorPersister.Persist(contexts).ConfigureAwait(false); + + using (bulkInsertDurationMeter.Measure()) + { + // not really interested in the batch results since a batch is atomic + await store.AsyncDatabaseCommands.BatchAsync(commands) + .ConfigureAwait(false); + } + + return storedContexts; + } + catch (Exception e) + { + if (log.IsDebugEnabled) + { + log.Debug("Bulk insertion failed", e); + } + + // making sure to rethrow so that all messages get marked as failed + throw; + } + finally + { + stopwatch.Stop(); + if (log.IsDebugEnabled) + { + log.Debug($"Batch size {contexts.Count} took {stopwatch.ElapsedMilliseconds} ms"); + } + } + } + Task Forward(IReadOnlyCollection messageContexts) { var transportOperations = new TransportOperation[messageContexts.Count]; //We could allocate based on the actual number of ProcessedMessages but this should be OK @@ -128,6 +175,8 @@ async Task VerifyCanReachForwardingAddress() IDispatchMessages dispatcher; string errorLogQueue; FailedMessageAnnouncer failedMessageAnnouncer; + readonly IDocumentStore store; + readonly Meter bulkInsertDurationMeter; ErrorPersister errorPersister; static ILog log = LogManager.GetLogger(); } diff --git a/src/ServiceControl/Operations/ErrorPersister.cs b/src/ServiceControl/Operations/ErrorPersister.cs index a1aba219a5..58fd7b12e2 100644 --- a/src/ServiceControl/Operations/ErrorPersister.cs +++ b/src/ServiceControl/Operations/ErrorPersister.cs @@ -53,82 +53,47 @@ public ErrorPersister(IDocumentStore store, BodyStorageEnricher bodyStorageEnric failedMessageFactory = new FailedMessageFactory(failedMessageEnrichers); } - public async Task> Persist(List contexts) + public async Task<(IReadOnlyList, IReadOnlyCollection)> Persist(List contexts) { - var stopwatch = Stopwatch.StartNew(); - - if (Logger.IsDebugEnabled) - { - Logger.Debug($"Batch size {contexts.Count}"); - } - var storedContexts = new List(contexts.Count); - try + var tasks = new List(contexts.Count); + foreach (var context in contexts) { - var tasks = new List(contexts.Count); - foreach (var context in contexts) - { - tasks.Add(ProcessMessage(context)); - } + tasks.Add(ProcessMessage(context)); + } - await Task.WhenAll(tasks).ConfigureAwait(false); + await Task.WhenAll(tasks).ConfigureAwait(false); - var commands = new List(contexts.Count); - var knownEndpoints = new Dictionary(); - foreach (var context in contexts) + var commands = new List(contexts.Count); + var knownEndpoints = new Dictionary(); + foreach (var context in contexts) + { + if (!context.Extensions.TryGet(out var command)) { - if (!context.Extensions.TryGet(out var command)) - { - continue; - } - - commands.Add(command); - storedContexts.Add(context); - ingestedCounter.Mark(); - - foreach (var endpointDetail in context.Extensions.Get>()) - { - RecordKnownEndpoints(endpointDetail, knownEndpoints); - } + continue; } - foreach (var endpoint in knownEndpoints.Values) - { - if (Logger.IsDebugEnabled) - { - Logger.Debug($"Adding known endpoint '{endpoint.EndpointDetails.Name}' for bulk storage"); - } - - commands.Add(CreateKnownEndpointsPutCommand(endpoint)); - } + commands.Add(command); + storedContexts.Add(context); + ingestedCounter.Mark(); - using (bulkInsertDurationMeter.Measure()) + foreach (var endpointDetail in context.Extensions.Get>()) { - // not really interested in the batch results since a batch is atomic - await store.AsyncDatabaseCommands.BatchAsync(commands) - .ConfigureAwait(false); + RecordKnownEndpoints(endpointDetail, knownEndpoints); } } - catch (Exception e) - { - if (Logger.IsDebugEnabled) - { - Logger.Debug("Bulk insertion failed", e); - } - // making sure to rethrow so that all messages get marked as failed - throw; - } - finally + foreach (var endpoint in knownEndpoints.Values) { - stopwatch.Stop(); if (Logger.IsDebugEnabled) { - Logger.Debug($"Batch size {contexts.Count} took {stopwatch.ElapsedMilliseconds} ms"); + Logger.Debug($"Adding known endpoint '{endpoint.EndpointDetails.Name}' for bulk storage"); } + + commands.Add(CreateKnownEndpointsPutCommand(endpoint)); } - return storedContexts; + return (storedContexts, commands); } static PutCommandData CreateKnownEndpointsPutCommand(KnownEndpoint endpoint) => From b07f02a5ad940b02b84ad61cb038b811df62644b Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 22 Jun 2021 12:09:04 +0200 Subject: [PATCH 03/42] Minor refactorings of the retry code --- .../Operations/ErrorIngestionComponent.cs | 11 ++- .../Operations/ErrorIngestor.cs | 60 +++++++++----- .../{ErrorPersister.cs => ErrorProcessor.cs} | 58 +++++++++---- .../Operations/FailedMessageAnnouncer.cs | 41 ---------- .../Operations/RetryConfirmationProcessor.cs | 82 +++++++++++++++++++ 5 files changed, 169 insertions(+), 83 deletions(-) rename src/ServiceControl/Operations/{ErrorPersister.cs => ErrorProcessor.cs} (83%) delete mode 100644 src/ServiceControl/Operations/FailedMessageAnnouncer.cs create mode 100644 src/ServiceControl/Operations/RetryConfirmationProcessor.cs diff --git a/src/ServiceControl/Operations/ErrorIngestionComponent.cs b/src/ServiceControl/Operations/ErrorIngestionComponent.cs index 98edefa1a7..b6dc802c8c 100644 --- a/src/ServiceControl/Operations/ErrorIngestionComponent.cs +++ b/src/ServiceControl/Operations/ErrorIngestionComponent.cs @@ -26,8 +26,6 @@ class ErrorIngestionComponent Settings settings; Channel channel; ErrorIngestor ingestor; - ErrorPersister persister; - Counter receivedMeter; Meter batchSizeMeter; Meter batchDurationMeter; @@ -44,16 +42,17 @@ public ErrorIngestionComponent( ErrorIngestionCustomCheck.State ingestionState ) { - receivedMeter = metrics.GetCounter("Error ingestion - received"); + var receivedMeter = metrics.GetCounter("Error ingestion - received"); batchSizeMeter = metrics.GetMeter("Error ingestion - batch size"); var ingestedMeter = metrics.GetCounter("Error ingestion - ingested"); var bulkInsertDurationMeter = metrics.GetMeter("Error ingestion - bulk insert duration", FrequencyInMilliseconds); batchDurationMeter = metrics.GetMeter("Error ingestion - batch processing duration", FrequencyInMilliseconds); this.settings = settings; - var announcer = new FailedMessageAnnouncer(domainEvents); - persister = new ErrorPersister(documentStore, bodyStorageEnricher, enrichers, failedMessageEnrichers, ingestedMeter, bulkInsertDurationMeter); - ingestor = new ErrorIngestor(persister, announcer, documentStore, bulkInsertDurationMeter, settings.ForwardErrorMessages, settings.ErrorLogQueue); + var errorProcessor = new ErrorProcessor(bodyStorageEnricher, enrichers, failedMessageEnrichers, domainEvents, ingestedMeter); + var retryConfirmationProcessor = new RetryConfirmationProcessor(domainEvents); + + ingestor = new ErrorIngestor(errorProcessor, retryConfirmationProcessor, documentStore, bulkInsertDurationMeter, settings.ForwardErrorMessages, settings.ErrorLogQueue); var ingestion = new ErrorIngestion(async messageContext => { diff --git a/src/ServiceControl/Operations/ErrorIngestor.cs b/src/ServiceControl/Operations/ErrorIngestor.cs index 7500c14b71..5a9c7194fc 100644 --- a/src/ServiceControl/Operations/ErrorIngestor.cs +++ b/src/ServiceControl/Operations/ErrorIngestor.cs @@ -3,8 +3,8 @@ using System; using System.Collections.Generic; using System.Diagnostics; + using System.Linq; using System.Threading.Tasks; - using Contracts.Operations; using Infrastructure.Metrics; using NServiceBus.Extensibility; using NServiceBus.Logging; @@ -14,10 +14,10 @@ class ErrorIngestor { - public ErrorIngestor(ErrorPersister errorPersister, FailedMessageAnnouncer failedMessageAnnouncer, IDocumentStore store, Meter bulkInsertDurationMeter, bool forwardErrorMessages, string errorLogQueue) + public ErrorIngestor(ErrorProcessor errorProcessor, RetryConfirmationProcessor retryConfirmationProcessor, IDocumentStore store, Meter bulkInsertDurationMeter, bool forwardErrorMessages, string errorLogQueue) { - this.errorPersister = errorPersister; - this.failedMessageAnnouncer = failedMessageAnnouncer; + this.errorProcessor = errorProcessor; + this.retryConfirmationProcessor = retryConfirmationProcessor; this.store = store; this.bulkInsertDurationMeter = bulkInsertDurationMeter; this.forwardErrorMessages = forwardErrorMessages; @@ -26,15 +26,35 @@ public ErrorIngestor(ErrorPersister errorPersister, FailedMessageAnnouncer faile public async Task Ingest(List contexts) { - var stored = await Persist(contexts) + var failedMessages = new List(contexts.Count); + var retriedMessages = new List(contexts.Count); + + foreach (MessageContext context in contexts) + { + if (context.Headers.ContainsKey(RetryConfirmationProcessor.SuccessfulRetryHeader)) + { + retriedMessages.Add(context); + } + else + { + failedMessages.Add(context); + } + } + + + var (storedFailed, storedRetried) = await PersistFailedMessages(failedMessages, retriedMessages) .ConfigureAwait(false); try { - var announcerTasks = new List(stored.Count); - foreach (var context in stored) + var announcerTasks = new List(contexts.Count); + foreach (var context in storedFailed) + { + announcerTasks.Add(errorProcessor.Announce(context)); + } + foreach (var context in storedRetried) { - announcerTasks.Add(failedMessageAnnouncer.Announce(context.Headers, context.Extensions.Get())); + announcerTasks.Add(retryConfirmationProcessor.Announce(context)); } await Task.WhenAll(announcerTasks).ConfigureAwait(false); @@ -45,14 +65,14 @@ public async Task Ingest(List contexts) { log.Debug($"Forwarding {contexts.Count} messages"); } - await Forward(stored).ConfigureAwait(false); + await Forward(storedFailed).ConfigureAwait(false); if (log.IsDebugEnabled) { - log.Debug($"Forwarded messages"); + log.Debug("Forwarded messages"); } } - foreach (var context in stored) + foreach (var context in storedFailed.Concat(storedRetried)) { context.GetTaskCompletionSource().TrySetResult(true); } @@ -69,27 +89,29 @@ public async Task Ingest(List contexts) } } - public async Task> Persist(List contexts) + async Task<(IReadOnlyList, IReadOnlyList)> PersistFailedMessages(List failedMessageContexts, List retriedMessageContexts) { var stopwatch = Stopwatch.StartNew(); if (log.IsDebugEnabled) { - log.Debug($"Batch size {contexts.Count}"); + log.Debug($"Batch size {failedMessageContexts.Count}"); } try { - var (storedContexts, commands) = await errorPersister.Persist(contexts).ConfigureAwait(false); + var (storedFailedMessageContexts, storeCommands) = await errorProcessor.Process(failedMessageContexts).ConfigureAwait(false); + var (storedRetriedMessageContexts, markRetriedCommands) = retryConfirmationProcessor.Process(retriedMessageContexts); using (bulkInsertDurationMeter.Measure()) { // not really interested in the batch results since a batch is atomic - await store.AsyncDatabaseCommands.BatchAsync(commands) + var allCommands = storeCommands.Concat(markRetriedCommands); + await store.AsyncDatabaseCommands.BatchAsync(allCommands) .ConfigureAwait(false); } - return storedContexts; + return (storedFailedMessageContexts, storedRetriedMessageContexts); } catch (Exception e) { @@ -106,7 +128,7 @@ await store.AsyncDatabaseCommands.BatchAsync(commands) stopwatch.Stop(); if (log.IsDebugEnabled) { - log.Debug($"Batch size {contexts.Count} took {stopwatch.ElapsedMilliseconds} ms"); + log.Debug($"Batch size {failedMessageContexts.Count} took {stopwatch.ElapsedMilliseconds} ms"); } } } @@ -174,10 +196,10 @@ async Task VerifyCanReachForwardingAddress() bool forwardErrorMessages; IDispatchMessages dispatcher; string errorLogQueue; - FailedMessageAnnouncer failedMessageAnnouncer; readonly IDocumentStore store; readonly Meter bulkInsertDurationMeter; - ErrorPersister errorPersister; + ErrorProcessor errorProcessor; + readonly RetryConfirmationProcessor retryConfirmationProcessor; static ILog log = LogManager.GetLogger(); } } \ No newline at end of file diff --git a/src/ServiceControl/Operations/ErrorPersister.cs b/src/ServiceControl/Operations/ErrorProcessor.cs similarity index 83% rename from src/ServiceControl/Operations/ErrorPersister.cs rename to src/ServiceControl/Operations/ErrorProcessor.cs index 58fd7b12e2..fbfa6a9120 100644 --- a/src/ServiceControl/Operations/ErrorPersister.cs +++ b/src/ServiceControl/Operations/ErrorProcessor.cs @@ -2,11 +2,12 @@ { using System; using System.Collections.Generic; - using System.Diagnostics; using System.Threading.Tasks; using BodyStorage; + using Contracts.MessageFailures; using Contracts.Operations; using Infrastructure; + using Infrastructure.DomainEvents; using Infrastructure.Metrics; using MessageFailures; using Monitoring; @@ -16,15 +17,14 @@ using Raven.Abstractions.Commands; using Raven.Abstractions.Data; using Raven.Abstractions.Extensions; - using Raven.Client; using Raven.Imports.Newtonsoft.Json; using Raven.Json.Linq; using Recoverability; using JsonSerializer = Raven.Imports.Newtonsoft.Json.JsonSerializer; - class ErrorPersister + class ErrorProcessor { - static ErrorPersister() + static ErrorProcessor() { Serializer = JsonExtensions.CreateDefaultJsonSerializer(); Serializer.TypeNameHandling = TypeNameHandling.Auto; @@ -42,18 +42,17 @@ static ErrorPersister() }}"); } - public ErrorPersister(IDocumentStore store, BodyStorageEnricher bodyStorageEnricher, IEnrichImportedErrorMessages[] enrichers, IFailedMessageEnricher[] failedMessageEnrichers, - Counter ingestedCounter, Meter bulkInsertDurationMeter) + public ErrorProcessor(BodyStorageEnricher bodyStorageEnricher, IEnrichImportedErrorMessages[] enrichers, IFailedMessageEnricher[] failedMessageEnrichers, IDomainEvents domainEvents, + Counter ingestedCounter) { - this.store = store; this.bodyStorageEnricher = bodyStorageEnricher; this.enrichers = enrichers; + this.domainEvents = domainEvents; this.ingestedCounter = ingestedCounter; - this.bulkInsertDurationMeter = bulkInsertDurationMeter; failedMessageFactory = new FailedMessageFactory(failedMessageEnrichers); } - public async Task<(IReadOnlyList, IReadOnlyCollection)> Persist(List contexts) + public async Task<(IReadOnlyList, IReadOnlyCollection)> Process(List contexts) { var storedContexts = new List(contexts.Count); var tasks = new List(contexts.Count); @@ -96,6 +95,32 @@ public ErrorPersister(IDocumentStore store, BodyStorageEnricher bodyStorageEnric return (storedContexts, commands); } + public Task Announce(MessageContext messageContext) + { + var failureDetails = messageContext.Extensions.Get(); + var headers = messageContext.Headers; + + var failingEndpointId = headers.ProcessingEndpointName(); + + if (headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var failedMessageId)) + { + return domainEvents.Raise(new MessageFailed + { + FailureDetails = failureDetails, + EndpointId = failingEndpointId, + FailedMessageId = failedMessageId, + RepeatedFailure = true + }); + } + + return domainEvents.Raise(new MessageFailed + { + FailureDetails = failureDetails, + EndpointId = failingEndpointId, + FailedMessageId = headers.UniqueId() + }); + } + static PutCommandData CreateKnownEndpointsPutCommand(KnownEndpoint endpoint) => new PutCommandData { @@ -155,7 +180,7 @@ await bodyStorageEnricher.StoreErrorMessageBody(context.Body, processingAttempt) { if (Logger.IsDebugEnabled) { - Logger.Debug($"Processing of message '{messageId}' failed.", e); + Logger.Debug($"Processing of message '{context.MessageId}' failed.", e); } context.GetTaskCompletionSource().TrySetException(e); @@ -226,15 +251,14 @@ static void RecordKnownEndpoints(EndpointDetails observedEndpoint, Dictionary(); + static readonly RavenJObject FailedMessageMetadata; + static readonly RavenJObject KnownEndpointMetadata; + static readonly JsonSerializer Serializer; + static readonly ILog Logger = LogManager.GetLogger(); } } \ No newline at end of file diff --git a/src/ServiceControl/Operations/FailedMessageAnnouncer.cs b/src/ServiceControl/Operations/FailedMessageAnnouncer.cs deleted file mode 100644 index 44c597d7e8..0000000000 --- a/src/ServiceControl/Operations/FailedMessageAnnouncer.cs +++ /dev/null @@ -1,41 +0,0 @@ -namespace ServiceControl.Operations -{ - using System.Collections.Generic; - using System.Threading.Tasks; - using Contracts.MessageFailures; - using Contracts.Operations; - using Infrastructure.DomainEvents; - - class FailedMessageAnnouncer - { - public FailedMessageAnnouncer(IDomainEvents domainEvents) - { - this.domainEvents = domainEvents; - } - - public Task Announce(IReadOnlyDictionary headers, FailureDetails failureDetails) - { - var failingEndpointId = headers.ProcessingEndpointName(); - - if (headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var failedMessageId)) - { - return domainEvents.Raise(new MessageFailed - { - FailureDetails = failureDetails, - EndpointId = failingEndpointId, - FailedMessageId = failedMessageId, - RepeatedFailure = true - }); - } - - return domainEvents.Raise(new MessageFailed - { - FailureDetails = failureDetails, - EndpointId = failingEndpointId, - FailedMessageId = headers.UniqueId() - }); - } - - IDomainEvents domainEvents; - } -} \ No newline at end of file diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs new file mode 100644 index 0000000000..10e1a7f4f5 --- /dev/null +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -0,0 +1,82 @@ +namespace ServiceControl.Operations +{ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + using Contracts.MessageFailures; + using Infrastructure.DomainEvents; + using MessageFailures; + using NServiceBus.Logging; + using NServiceBus.Transport; + using Raven.Abstractions.Commands; + using Raven.Abstractions.Data; + using Recoverability; + + class RetryConfirmationProcessor + { + public const string SuccessfulRetryHeader = "ServiceControl.Retry.SuccessfulRetryUniqueMessageId"; + + public RetryConfirmationProcessor(IDomainEvents domainEvents) + { + this.domainEvents = domainEvents; + } + + public (IReadOnlyList, IReadOnlyCollection) Process(List contexts) + { + var storedContexts = new List(contexts.Count); + var allCommands = new List(contexts.Count); + + foreach (var context in contexts) + { + try + { + var commands = ProcessOne(context); + allCommands.AddRange(commands); + storedContexts.Add(context); + } + catch (Exception e) + { + if (Logger.IsDebugEnabled) + { + Logger.Debug($"Processing of message '{context.MessageId}' failed.", e); + } + + context.GetTaskCompletionSource().TrySetException(e); + } + } + + return (storedContexts, allCommands); + } + + public Task Announce(MessageContext messageContext) + { + return domainEvents.Raise(new MessageFailureResolvedByRetry + { + FailedMessageId = messageContext.Headers[SuccessfulRetryHeader] + }); + } + + static IEnumerable ProcessOne(MessageContext context) + { + var retriedMessageUniqueId = context.Headers[SuccessfulRetryHeader]; + var failedMessageDocumentId = FailedMessage.MakeDocumentId(retriedMessageUniqueId); + var failedMessageRetryDocumentId = FailedMessageRetry.MakeDocumentId(retriedMessageUniqueId); + + var patchCommand = new PatchCommandData + { + Key = failedMessageDocumentId, + Patches = new[] + { + new PatchRequest {Type = PatchCommandType.Set, Name = "status", Value = (int)FailedMessageStatus.Resolved} + } + }; + + var deleteCommand = new DeleteCommandData { Key = failedMessageRetryDocumentId, }; + yield return patchCommand; + yield return deleteCommand; + } + + static readonly ILog Logger = LogManager.GetLogger(); + readonly IDomainEvents domainEvents; + } +} \ No newline at end of file From 20bd9a6f4812280a589c0d09ea7487b783da18a5 Mon Sep 17 00:00:00 2001 From: Tim Bussmann Date: Wed, 23 Jun 2021 16:05:45 +0200 Subject: [PATCH 04/42] use constant for header key --- .../Operations/RetryConfirmationProcessor.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index 10e1a7f4f5..158d7f54d6 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -14,7 +14,8 @@ class RetryConfirmationProcessor { - public const string SuccessfulRetryHeader = "ServiceControl.Retry.SuccessfulRetryUniqueMessageId"; + public const string SuccessfulRetryHeader = "ServiceControl.Retry.Successful"; + const string RetryUniqueMessageIdHeader = "ServiceControl.Retry.UniqueMessageId"; public RetryConfirmationProcessor(IDomainEvents domainEvents) { @@ -52,13 +53,13 @@ public Task Announce(MessageContext messageContext) { return domainEvents.Raise(new MessageFailureResolvedByRetry { - FailedMessageId = messageContext.Headers[SuccessfulRetryHeader] + FailedMessageId = messageContext.Headers[RetryUniqueMessageIdHeader] }); } static IEnumerable ProcessOne(MessageContext context) { - var retriedMessageUniqueId = context.Headers[SuccessfulRetryHeader]; + var retriedMessageUniqueId = context.Headers[RetryUniqueMessageIdHeader]; var failedMessageDocumentId = FailedMessage.MakeDocumentId(retriedMessageUniqueId); var failedMessageRetryDocumentId = FailedMessageRetry.MakeDocumentId(retriedMessageUniqueId); From 25da52d03240066e54aa27010563cfa6d4875e65 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Thu, 24 Jun 2021 15:16:47 +0200 Subject: [PATCH 05/42] Include successful retries in all messages API --- .../CompositeViews/Messages/MessagesViewIndex.cs | 1 - .../Messages/ScatterGatherApiMessageView.cs | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs b/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs index 21db4083f2..91d3b8227d 100644 --- a/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs +++ b/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs @@ -31,7 +31,6 @@ public MessagesViewIndex() }); AddMap(messages => from message in messages - where message.Status != FailedMessageStatus.Resolved let last = message.ProcessingAttempts.Last() select new SortAndFilterOptions { diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs index f85658e29a..17a700ef3d 100644 --- a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs +++ b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs @@ -2,6 +2,7 @@ namespace ServiceControl.CompositeViews.Messages { using System; using System.Collections.Generic; + using System.Linq; using System.Net.Http; using Raven.Client; using ServiceBus.Management.Infrastructure.Settings; @@ -14,7 +15,7 @@ protected ScatterGatherApiMessageView(IDocumentStore documentStore, Settings set protected override IList ProcessResults(HttpRequestMessage request, QueryResult>[] results) { - var combined = new List(); + var deduplicated = new Dictionary(); foreach (var queryResult in results) { var messagesViews = queryResult?.Results ?? new List(); @@ -31,9 +32,17 @@ protected override IList ProcessResults(HttpRequestMessage request } } - combined.AddRange(messagesViews); + //HINT: De-duplicate the results as some messages might be present in multiple instances (e.g. when they initially failed and later were successfully processed) + foreach (MessagesView messagesView in messagesViews) + { + if (!deduplicated.ContainsKey(messagesView.MessageId)) + { + deduplicated.Add(messagesView.Id, messagesView); + } + } } + var combined = deduplicated.Values.ToList(); var comparer = FinalOrder(request); if (comparer != null) { From bbe9843cac9559159e6abd95cdac020472e2463f Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Thu, 24 Jun 2021 15:22:48 +0200 Subject: [PATCH 06/42] Do not update state twice --- .../MessageFailures/MessageFailureResolvedByRetry.cs | 1 + .../Handlers/MessageFailureResolvedDomainHandler.cs | 6 ++++++ src/ServiceControl/Operations/RetryConfirmationProcessor.cs | 3 ++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs index 32a7f83ed7..95d0e75e6f 100644 --- a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs @@ -7,6 +7,7 @@ // Comes from unconverted legacy instances public class MessageFailureResolvedByRetry : IDomainEvent, IMessage, IUserInterfaceEvent { + public bool StateUpdated { get; set; } public string FailedMessageId { get; set; } public string[] AlternativeFailedMessageIds { get; set; } } diff --git a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs index d969bef8d5..ac3f60d236 100644 --- a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs +++ b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs @@ -18,6 +18,12 @@ public MessageFailureResolvedDomainHandler(IDocumentStore store, RetryDocumentMa public async Task Handle(MessageFailureResolvedByRetry domainEvent) { + //HINT: This event has been published when the SC main instance received successful retry confirmation and the state has been updated via patch commands generated by RetryConfirmationProcessor + if (domainEvent.StateUpdated) + { + return; + } + if (await MarkMessageAsResolved(domainEvent.FailedMessageId) .ConfigureAwait(false)) { diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index 158d7f54d6..ebc0e3b42c 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -53,7 +53,8 @@ public Task Announce(MessageContext messageContext) { return domainEvents.Raise(new MessageFailureResolvedByRetry { - FailedMessageId = messageContext.Headers[RetryUniqueMessageIdHeader] + FailedMessageId = messageContext.Headers[RetryUniqueMessageIdHeader], + StateUpdated = true }); } From ba04b03ab3bd23ca0629af9b3e9d6a95fd04b954 Mon Sep 17 00:00:00 2001 From: Tim Bussmann Date: Fri, 25 Jun 2021 15:48:28 +0200 Subject: [PATCH 07/42] fix property casing --- src/ServiceControl/Operations/RetryConfirmationProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index ebc0e3b42c..0e8824822b 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -69,7 +69,7 @@ static IEnumerable ProcessOne(MessageContext context) Key = failedMessageDocumentId, Patches = new[] { - new PatchRequest {Type = PatchCommandType.Set, Name = "status", Value = (int)FailedMessageStatus.Resolved} + new PatchRequest {Type = PatchCommandType.Set, Name = "Status", Value = (int)FailedMessageStatus.Resolved} } }; From f1314fdb7e49273651d940fe067d4c7c58294b64 Mon Sep 17 00:00:00 2001 From: Tim Bussmann Date: Fri, 25 Jun 2021 17:32:41 +0200 Subject: [PATCH 08/42] update messages view to show successful retries without auditing --- .../CompositeViews/MessagesViewTests.cs | 39 +++++++- .../Messages/MessagesViewIndex.cs | 8 +- .../Messages/MessagesViewTransformer.cs | 92 ++++++++++--------- 3 files changed, 91 insertions(+), 48 deletions(-) diff --git a/src/ServiceControl.UnitTests/CompositeViews/MessagesViewTests.cs b/src/ServiceControl.UnitTests/CompositeViews/MessagesViewTests.cs index 9ab8f614f9..1669d92607 100644 --- a/src/ServiceControl.UnitTests/CompositeViews/MessagesViewTests.cs +++ b/src/ServiceControl.UnitTests/CompositeViews/MessagesViewTests.cs @@ -203,6 +203,44 @@ public void TimeSent_is_not_cast_to_DateTimeMin_if_null() } } + [TestCase(FailedMessageStatus.Archived, MessageStatus.ArchivedFailure)] + [TestCase(FailedMessageStatus.Resolved, MessageStatus.ResolvedSuccessfully)] + [TestCase(FailedMessageStatus.RetryIssued, MessageStatus.RetryIssued)] + [TestCase(FailedMessageStatus.Unresolved, MessageStatus.Failed)] + public void Correct_status_for_failed_messages(FailedMessageStatus failedMessageStatus, MessageStatus expecteMessageStatus) + { + using (var session = documentStore.OpenSession()) + { + session.Store(new FailedMessage + { + Id = "1", + ProcessingAttempts = new List + { + new FailedMessage.ProcessingAttempt + { + AttemptedAt = DateTime.Today, + MessageMetadata = new Dictionary {{"MessageIntent", "1"}} + } + }, + Status = failedMessageStatus + }); + + session.SaveChanges(); + } + + documentStore.WaitForIndexing(); + + using (var session = documentStore.OpenSession()) + { + var message = session.Query() + .TransformWith() + .Customize(x => x.WaitForNonStaleResults()) + .Single(); + + Assert.AreEqual(expecteMessageStatus, message.Status); + } + } + [Test] public void Correct_status_for_repeated_errors() { @@ -242,7 +280,6 @@ public void Correct_status_for_repeated_errors() } } - [SetUp] public void SetUp() { diff --git a/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs b/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs index 91d3b8227d..f52de4ed9d 100644 --- a/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs +++ b/src/ServiceControl/CompositeViews/Messages/MessagesViewIndex.cs @@ -39,9 +39,11 @@ public MessagesViewIndex() IsSystemMessage = (bool)last.MessageMetadata["IsSystemMessage"], Status = message.Status == FailedMessageStatus.Archived ? MessageStatus.ArchivedFailure - : message.ProcessingAttempts.Count == 1 - ? MessageStatus.Failed - : MessageStatus.RepeatedFailure, + : message.Status == FailedMessageStatus.Resolved + ? MessageStatus.ResolvedSuccessfully + : message.ProcessingAttempts.Count == 1 + ? MessageStatus.Failed + : MessageStatus.RepeatedFailure, TimeSent = (DateTime)last.MessageMetadata["TimeSent"], ProcessedAt = last.AttemptedAt, ReceivingEndpointName = ((EndpointDetails)last.MessageMetadata["ReceivingEndpoint"]).Name, diff --git a/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs b/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs index 68bd07e7c3..a045078efc 100644 --- a/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs +++ b/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs @@ -12,50 +12,54 @@ public class MessagesViewTransformer : AbstractTransformerCreationTask from message in messages - let metadata = - message.ProcessingAttempts != null - ? message.ProcessingAttempts.Last().MessageMetadata - : message.MessageMetadata - let headers = - message.ProcessingAttempts != null ? message.ProcessingAttempts.Last().Headers : message.Headers - let processedAt = - message.ProcessingAttempts != null - ? message.ProcessingAttempts.Last().AttemptedAt - : message.ProcessedAt - let status = - message.ProcessingAttempts == null - ? !(bool)message.MessageMetadata["IsRetried"] ? MessageStatus.Successful : MessageStatus.ResolvedSuccessfully - : message.Status == FailedMessageStatus.RetryIssued - ? MessageStatus.RetryIssued - : message.Status == FailedMessageStatus.Archived - ? MessageStatus.ArchivedFailure - : message.ProcessingAttempts.Count == 1 - ? MessageStatus.Failed - : MessageStatus.RepeatedFailure - select new - { - Id = message.UniqueMessageId, - MessageId = metadata["MessageId"], - MessageType = metadata["MessageType"], - SendingEndpoint = metadata["SendingEndpoint"], - ReceivingEndpoint = metadata["ReceivingEndpoint"], - TimeSent = (DateTime?)metadata["TimeSent"], - ProcessedAt = processedAt, - CriticalTime = (TimeSpan)metadata["CriticalTime"], - ProcessingTime = (TimeSpan)metadata["ProcessingTime"], - DeliveryTime = (TimeSpan)metadata["DeliveryTime"], - IsSystemMessage = (bool)metadata["IsSystemMessage"], - ConversationId = metadata["ConversationId"], - //the reason the we need to use a KeyValuePair is that raven seems to interpret the values and convert them - // to real types. In this case it was the NServiceBus.Temporary.DelayDeliveryWith header to was converted to a timespan - Headers = headers.Select(header => new KeyValuePair(header.Key, header.Value)), - Status = status, - MessageIntent = metadata["MessageIntent"], - BodyUrl = metadata["BodyUrl"], - BodySize = (int)metadata["ContentLength"], - InvokedSagas = metadata["InvokedSagas"], - OriginatesFromSaga = metadata["OriginatesFromSaga"] - }; + let metadata = + message.ProcessingAttempts != null + ? message.ProcessingAttempts.Last().MessageMetadata + : message.MessageMetadata + let headers = + message.ProcessingAttempts != null ? message.ProcessingAttempts.Last().Headers : message.Headers + let processedAt = + message.ProcessingAttempts != null + ? message.ProcessingAttempts.Last().AttemptedAt + : message.ProcessedAt + let status = + message.ProcessingAttempts == null + ? !(bool)message.MessageMetadata["IsRetried"] + ? MessageStatus.Successful + : MessageStatus.ResolvedSuccessfully + : message.Status == FailedMessageStatus.Resolved + ? MessageStatus.ResolvedSuccessfully + : message.Status == FailedMessageStatus.RetryIssued + ? MessageStatus.RetryIssued + : message.Status == FailedMessageStatus.Archived + ? MessageStatus.ArchivedFailure + : message.ProcessingAttempts.Count == 1 + ? MessageStatus.Failed + : MessageStatus.RepeatedFailure + select new + { + Id = message.UniqueMessageId, + MessageId = metadata["MessageId"], + MessageType = metadata["MessageType"], + SendingEndpoint = metadata["SendingEndpoint"], + ReceivingEndpoint = metadata["ReceivingEndpoint"], + TimeSent = (DateTime?)metadata["TimeSent"], + ProcessedAt = processedAt, + CriticalTime = (TimeSpan)metadata["CriticalTime"], + ProcessingTime = (TimeSpan)metadata["ProcessingTime"], + DeliveryTime = (TimeSpan)metadata["DeliveryTime"], + IsSystemMessage = (bool)metadata["IsSystemMessage"], + ConversationId = metadata["ConversationId"], + //the reason the we need to use a KeyValuePair is that raven seems to interpret the values and convert them + // to real types. In this case it was the NServiceBus.Temporary.DelayDeliveryWith header to was converted to a timespan + Headers = headers.Select(header => new KeyValuePair(header.Key, header.Value)), + Status = status, + MessageIntent = metadata["MessageIntent"], + BodyUrl = metadata["BodyUrl"], + BodySize = (int)metadata["ContentLength"], + InvokedSagas = metadata["InvokedSagas"], + OriginatesFromSaga = metadata["OriginatesFromSaga"] + }; } public class Input From 47f19cc6d6493189763ebddccc787f26ca80016e Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 29 Jun 2021 12:08:21 +0200 Subject: [PATCH 09/42] Update the failed message document in the message handler --- .../MessageFailureResolvedByRetry.cs | 5 +- ...essageFailureResolvedByRetryDomainEvent.cs | 11 +++ ...MessageFailureResolvedByRetryDefinition.cs | 2 +- .../HandleLegacyFailureResolvedByRetry.cs | 23 ------ .../MessageFailureResolvedDomainHandler.cs | 75 ------------------- .../Handlers/MessageFailureResolvedHandler.cs | 66 ++++++++++++++-- .../Operations/RetryConfirmationProcessor.cs | 3 +- .../MessageFailureResolvedByRetryPublisher.cs | 4 +- .../Recoverability/RecoverabilityComponent.cs | 2 - 9 files changed, 75 insertions(+), 116 deletions(-) create mode 100644 src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs delete mode 100644 src/ServiceControl/MessageFailures/Handlers/HandleLegacyFailureResolvedByRetry.cs delete mode 100644 src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs index 95d0e75e6f..eceb3888f3 100644 --- a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs @@ -1,13 +1,10 @@ namespace ServiceControl.Contracts.MessageFailures { - using Infrastructure.DomainEvents; - using Infrastructure.SignalR; using NServiceBus; // Comes from unconverted legacy instances - public class MessageFailureResolvedByRetry : IDomainEvent, IMessage, IUserInterfaceEvent + public class MessageFailureResolvedByRetry : IMessage { - public bool StateUpdated { get; set; } public string FailedMessageId { get; set; } public string[] AlternativeFailedMessageIds { get; set; } } diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs new file mode 100644 index 0000000000..34640e8fcf --- /dev/null +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs @@ -0,0 +1,11 @@ +namespace ServiceControl.Contracts.MessageFailures +{ + using Infrastructure.DomainEvents; + using Infrastructure.SignalR; + + public class MessageFailureResolvedByRetryDomainEvent : IDomainEvent, IUserInterfaceEvent + { + public string FailedMessageId { get; set; } + public string[] AlternativeFailedMessageIds { get; set; } + } +} \ No newline at end of file diff --git a/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs b/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs index 23f31de096..93d3125303 100644 --- a/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs +++ b/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs @@ -2,7 +2,7 @@ { using Contracts.MessageFailures; - class MessageFailureResolvedByRetryDefinition : EventLogMappingDefinition + class MessageFailureResolvedByRetryDefinition : EventLogMappingDefinition { public MessageFailureResolvedByRetryDefinition() { diff --git a/src/ServiceControl/MessageFailures/Handlers/HandleLegacyFailureResolvedByRetry.cs b/src/ServiceControl/MessageFailures/Handlers/HandleLegacyFailureResolvedByRetry.cs deleted file mode 100644 index b0f03987a0..0000000000 --- a/src/ServiceControl/MessageFailures/Handlers/HandleLegacyFailureResolvedByRetry.cs +++ /dev/null @@ -1,23 +0,0 @@ -namespace ServiceControl.MessageFailures.Handlers -{ - using System.Threading.Tasks; - using Contracts.MessageFailures; - using Infrastructure.DomainEvents; - using NServiceBus; - - class HandleLegacyFailureResolvedByRetry : IHandleMessages - { - public HandleLegacyFailureResolvedByRetry(IDomainEvents domainEvents) - { - this.domainEvents = domainEvents; - } - - // This is only needed because we might get this from legacy not yet converted instances - public Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerContext context) - { - return domainEvents.Raise(message); - } - - readonly IDomainEvents domainEvents; - } -} \ No newline at end of file diff --git a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs deleted file mode 100644 index ac3f60d236..0000000000 --- a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedDomainHandler.cs +++ /dev/null @@ -1,75 +0,0 @@ -namespace ServiceControl.MessageFailures.Handlers -{ - using System; - using System.Linq; - using System.Threading.Tasks; - using Contracts.MessageFailures; - using Infrastructure.DomainEvents; - using Raven.Client; - using Recoverability; - - class MessageFailureResolvedDomainHandler : IDomainHandler - { - public MessageFailureResolvedDomainHandler(IDocumentStore store, RetryDocumentManager retryDocumentManager) - { - this.store = store; - this.retryDocumentManager = retryDocumentManager; - } - - public async Task Handle(MessageFailureResolvedByRetry domainEvent) - { - //HINT: This event has been published when the SC main instance received successful retry confirmation and the state has been updated via patch commands generated by RetryConfirmationProcessor - if (domainEvent.StateUpdated) - { - return; - } - - if (await MarkMessageAsResolved(domainEvent.FailedMessageId) - .ConfigureAwait(false)) - { - return; - } - - if (domainEvent.AlternativeFailedMessageIds == null) - { - return; - } - - foreach (var alternative in domainEvent.AlternativeFailedMessageIds.Where(x => x != domainEvent.FailedMessageId)) - { - if (await MarkMessageAsResolved(alternative) - .ConfigureAwait(false)) - { - return; - } - } - } - - async Task MarkMessageAsResolved(string failedMessageId) - { - await retryDocumentManager.RemoveFailedMessageRetryDocument(failedMessageId).ConfigureAwait(false); - - using (var session = store.OpenAsyncSession()) - { - session.Advanced.UseOptimisticConcurrency = true; - - var failedMessage = await session.LoadAsync(new Guid(failedMessageId)) - .ConfigureAwait(false); - - if (failedMessage == null) - { - return false; - } - - failedMessage.Status = FailedMessageStatus.Resolved; - - await session.SaveChangesAsync().ConfigureAwait(false); - - return true; - } - } - - IDocumentStore store; - RetryDocumentManager retryDocumentManager; - } -} \ No newline at end of file diff --git a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs index db54c84378..44e55efb7e 100644 --- a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs +++ b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs @@ -1,6 +1,7 @@ namespace ServiceControl.MessageFailures.Handlers { using System; + using System.Linq; using System.Threading.Tasks; using Api; using Contracts.MessageFailures; @@ -8,25 +9,73 @@ using InternalMessages; using NServiceBus; using Raven.Client; + using Recoverability; class MessageFailureResolvedHandler : IHandleMessages, + IHandleMessages, IHandleMessages, IHandleMessages { - public MessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainEvents) + public MessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainEvents, RetryDocumentManager retryDocumentManager) { this.store = store; this.domainEvents = domainEvents; + this.retryDocumentManager = retryDocumentManager; } - public Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHandlerContext context) + public async Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHandlerContext context) { - return domainEvents.Raise(new MessageFailureResolvedByRetry + var primaryId = message.FailedMessageId; + var messageAlternativeFailedMessageIds = message.AlternativeFailedMessageIds; + + await MarkAsResolvedByRetry(primaryId, messageAlternativeFailedMessageIds) + .ConfigureAwait(false); + await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent { AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, FailedMessageId = message.FailedMessageId - }); + }).ConfigureAwait(false); + } + + // This is only needed because we might get this from legacy not yet converted instances + public async Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerContext context) + { + var primaryId = message.FailedMessageId; + var messageAlternativeFailedMessageIds = message.AlternativeFailedMessageIds; + + await MarkAsResolvedByRetry(primaryId, messageAlternativeFailedMessageIds) + .ConfigureAwait(false); + await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + { + AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, + FailedMessageId = message.FailedMessageId + }).ConfigureAwait(false); + } + + async Task MarkAsResolvedByRetry(string primaryId, string[] messageAlternativeFailedMessageIds) + { + await retryDocumentManager.RemoveFailedMessageRetryDocument(primaryId).ConfigureAwait(false); + if (await MarkMessageAsResolved(primaryId) + .ConfigureAwait(false)) + { + return; + } + + if (messageAlternativeFailedMessageIds == null) + { + return; + } + + foreach (var alternative in messageAlternativeFailedMessageIds.Where(x => x != primaryId)) + { + await retryDocumentManager.RemoveFailedMessageRetryDocument(alternative).ConfigureAwait(false); + if (await MarkMessageAsResolved(alternative) + .ConfigureAwait(false)) + { + return; + } + } } public async Task Handle(MarkPendingRetriesAsResolved message, IMessageHandlerContext context) @@ -71,8 +120,9 @@ await domainEvents.Raise(new MessageFailureResolvedManually }).ConfigureAwait(false); } - async Task MarkMessageAsResolved(string failedMessageId) + async Task MarkMessageAsResolved(string failedMessageId) { + using (var session = store.OpenAsyncSession()) { session.Advanced.UseOptimisticConcurrency = true; @@ -82,17 +132,19 @@ async Task MarkMessageAsResolved(string failedMessageId) if (failedMessage == null) { - return; + return false; } failedMessage.Status = FailedMessageStatus.Resolved; await session.SaveChangesAsync().ConfigureAwait(false); + + return true; } } IDocumentStore store; - IDomainEvents domainEvents; + RetryDocumentManager retryDocumentManager; } } \ No newline at end of file diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index 0e8824822b..c6c53b2099 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -51,10 +51,9 @@ public RetryConfirmationProcessor(IDomainEvents domainEvents) public Task Announce(MessageContext messageContext) { - return domainEvents.Raise(new MessageFailureResolvedByRetry + return domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent { FailedMessageId = messageContext.Headers[RetryUniqueMessageIdHeader], - StateUpdated = true }); } diff --git a/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs b/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs index a9fd5a1476..5f219f8633 100644 --- a/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs +++ b/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs @@ -7,9 +7,9 @@ namespace ServiceControl.Recoverability.ExternalIntegration using ExternalIntegrations; using Raven.Client; - class MessageFailureResolvedByRetryPublisher : EventPublisher + class MessageFailureResolvedByRetryPublisher : EventPublisher { - protected override DispatchContext CreateDispatchRequest(MessageFailureResolvedByRetry @event) + protected override DispatchContext CreateDispatchRequest(MessageFailureResolvedByRetryDomainEvent @event) { return new DispatchContext { diff --git a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs index 9a48749c80..aca4222c47 100644 --- a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs +++ b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs @@ -11,7 +11,6 @@ using Infrastructure.RavenDB; using MessageFailures; using MessageFailures.Api; - using MessageFailures.Handlers; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using NServiceBus.Logging; @@ -88,7 +87,6 @@ public override void Configure(Settings settings, IHostBuilder hostBuilder) //Failed messages collection.AddSingleton(); collection.AddHostedService(); - collection.AddDomainEventHandler(); //Body storage collection.AddSingleton(); From cc589c733562ea81e0138c545334d937f5798a2a Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 29 Jun 2021 12:23:50 +0200 Subject: [PATCH 10/42] Fix damn formatting. I hate whitespace police! --- .../Messages/MessagesViewTransformer.cs | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs b/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs index a045078efc..6c5f5199c5 100644 --- a/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs +++ b/src/ServiceControl/CompositeViews/Messages/MessagesViewTransformer.cs @@ -12,54 +12,54 @@ public class MessagesViewTransformer : AbstractTransformerCreationTask from message in messages - let metadata = + let metadata = message.ProcessingAttempts != null ? message.ProcessingAttempts.Last().MessageMetadata : message.MessageMetadata - let headers = - message.ProcessingAttempts != null ? message.ProcessingAttempts.Last().Headers : message.Headers - let processedAt = - message.ProcessingAttempts != null - ? message.ProcessingAttempts.Last().AttemptedAt - : message.ProcessedAt - let status = - message.ProcessingAttempts == null - ? !(bool)message.MessageMetadata["IsRetried"] - ? MessageStatus.Successful - : MessageStatus.ResolvedSuccessfully - : message.Status == FailedMessageStatus.Resolved - ? MessageStatus.ResolvedSuccessfully - : message.Status == FailedMessageStatus.RetryIssued - ? MessageStatus.RetryIssued - : message.Status == FailedMessageStatus.Archived - ? MessageStatus.ArchivedFailure - : message.ProcessingAttempts.Count == 1 - ? MessageStatus.Failed - : MessageStatus.RepeatedFailure - select new - { - Id = message.UniqueMessageId, - MessageId = metadata["MessageId"], - MessageType = metadata["MessageType"], - SendingEndpoint = metadata["SendingEndpoint"], - ReceivingEndpoint = metadata["ReceivingEndpoint"], - TimeSent = (DateTime?)metadata["TimeSent"], - ProcessedAt = processedAt, - CriticalTime = (TimeSpan)metadata["CriticalTime"], - ProcessingTime = (TimeSpan)metadata["ProcessingTime"], - DeliveryTime = (TimeSpan)metadata["DeliveryTime"], - IsSystemMessage = (bool)metadata["IsSystemMessage"], - ConversationId = metadata["ConversationId"], - //the reason the we need to use a KeyValuePair is that raven seems to interpret the values and convert them - // to real types. In this case it was the NServiceBus.Temporary.DelayDeliveryWith header to was converted to a timespan - Headers = headers.Select(header => new KeyValuePair(header.Key, header.Value)), - Status = status, - MessageIntent = metadata["MessageIntent"], - BodyUrl = metadata["BodyUrl"], - BodySize = (int)metadata["ContentLength"], - InvokedSagas = metadata["InvokedSagas"], - OriginatesFromSaga = metadata["OriginatesFromSaga"] - }; + let headers = + message.ProcessingAttempts != null ? message.ProcessingAttempts.Last().Headers : message.Headers + let processedAt = + message.ProcessingAttempts != null + ? message.ProcessingAttempts.Last().AttemptedAt + : message.ProcessedAt + let status = + message.ProcessingAttempts == null + ? !(bool)message.MessageMetadata["IsRetried"] + ? MessageStatus.Successful + : MessageStatus.ResolvedSuccessfully + : message.Status == FailedMessageStatus.Resolved + ? MessageStatus.ResolvedSuccessfully + : message.Status == FailedMessageStatus.RetryIssued + ? MessageStatus.RetryIssued + : message.Status == FailedMessageStatus.Archived + ? MessageStatus.ArchivedFailure + : message.ProcessingAttempts.Count == 1 + ? MessageStatus.Failed + : MessageStatus.RepeatedFailure + select new + { + Id = message.UniqueMessageId, + MessageId = metadata["MessageId"], + MessageType = metadata["MessageType"], + SendingEndpoint = metadata["SendingEndpoint"], + ReceivingEndpoint = metadata["ReceivingEndpoint"], + TimeSent = (DateTime?)metadata["TimeSent"], + ProcessedAt = processedAt, + CriticalTime = (TimeSpan)metadata["CriticalTime"], + ProcessingTime = (TimeSpan)metadata["ProcessingTime"], + DeliveryTime = (TimeSpan)metadata["DeliveryTime"], + IsSystemMessage = (bool)metadata["IsSystemMessage"], + ConversationId = metadata["ConversationId"], + //the reason the we need to use a KeyValuePair is that raven seems to interpret the values and convert them + // to real types. In this case it was the NServiceBus.Temporary.DelayDeliveryWith header to was converted to a timespan + Headers = headers.Select(header => new KeyValuePair(header.Key, header.Value)), + Status = status, + MessageIntent = metadata["MessageIntent"], + BodyUrl = metadata["BodyUrl"], + BodySize = (int)metadata["ContentLength"], + InvokedSagas = metadata["InvokedSagas"], + OriginatesFromSaga = metadata["OriginatesFromSaga"] + }; } public class Input From 92e0be91b0d03c1463be976d7473cae12d874efd Mon Sep 17 00:00:00 2001 From: Tim Bussmann Date: Wed, 30 Jun 2021 14:54:02 +0200 Subject: [PATCH 11/42] minor tweaks --- .../Handlers/MessageFailureResolvedHandler.cs | 10 ++-------- src/ServiceControl/Operations/ErrorIngestor.cs | 2 +- .../Operations/RetryConfirmationProcessor.cs | 6 +++--- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs index 44e55efb7e..48b6d8edfe 100644 --- a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs +++ b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs @@ -26,10 +26,7 @@ public MessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainE public async Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHandlerContext context) { - var primaryId = message.FailedMessageId; - var messageAlternativeFailedMessageIds = message.AlternativeFailedMessageIds; - - await MarkAsResolvedByRetry(primaryId, messageAlternativeFailedMessageIds) + await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) .ConfigureAwait(false); await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent { @@ -41,10 +38,7 @@ await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent // This is only needed because we might get this from legacy not yet converted instances public async Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerContext context) { - var primaryId = message.FailedMessageId; - var messageAlternativeFailedMessageIds = message.AlternativeFailedMessageIds; - - await MarkAsResolvedByRetry(primaryId, messageAlternativeFailedMessageIds) + await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) .ConfigureAwait(false); await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent { diff --git a/src/ServiceControl/Operations/ErrorIngestor.cs b/src/ServiceControl/Operations/ErrorIngestor.cs index 5a9c7194fc..e4a58832ef 100644 --- a/src/ServiceControl/Operations/ErrorIngestor.cs +++ b/src/ServiceControl/Operations/ErrorIngestor.cs @@ -29,7 +29,7 @@ public async Task Ingest(List contexts) var failedMessages = new List(contexts.Count); var retriedMessages = new List(contexts.Count); - foreach (MessageContext context in contexts) + foreach (var context in contexts) { if (context.Headers.ContainsKey(RetryConfirmationProcessor.SuccessfulRetryHeader)) { diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index c6c53b2099..8bdc3b5965 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -31,7 +31,7 @@ public RetryConfirmationProcessor(IDomainEvents domainEvents) { try { - var commands = ProcessOne(context); + var commands = CreateDatabaseCommands(context); allCommands.AddRange(commands); storedContexts.Add(context); } @@ -57,7 +57,7 @@ public Task Announce(MessageContext messageContext) }); } - static IEnumerable ProcessOne(MessageContext context) + static IEnumerable CreateDatabaseCommands(MessageContext context) { var retriedMessageUniqueId = context.Headers[RetryUniqueMessageIdHeader]; var failedMessageDocumentId = FailedMessage.MakeDocumentId(retriedMessageUniqueId); @@ -68,7 +68,7 @@ static IEnumerable ProcessOne(MessageContext context) Key = failedMessageDocumentId, Patches = new[] { - new PatchRequest {Type = PatchCommandType.Set, Name = "Status", Value = (int)FailedMessageStatus.Resolved} + new PatchRequest {Type = PatchCommandType.Set, Name = nameof(FailedMessage.Status), Value = (int)FailedMessageStatus.Resolved} } }; From 7dbc2e2e842b99ee48b1d74421a46ac4e8061db6 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Thu, 1 Jul 2021 14:40:35 +0200 Subject: [PATCH 12/42] Do not send a command to mark a retry successful if a direct ack has been sent by the endpoint --- .../Recoverability/FailedMessagesFeature.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index a35afdc4d4..d657596090 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -28,12 +28,13 @@ public void Enrich(AuditEnricherContext context) var headers = context.Headers; var isOldRetry = headers.TryGetValue("ServiceControl.RetryId", out _); var isNewRetry = headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var newRetryMessageId); + var isAckHandled = headers.ContainsKey("ServiceControl.Retry.AcknowledgementSent"); var hasBeenRetried = isOldRetry || isNewRetry; context.Metadata.Add("IsRetried", hasBeenRetried); - if (!hasBeenRetried) + if (!hasBeenRetried || isAckHandled) { return; } From 00f4f6196a5c2bf93312ddb5e51c0885630eff72 Mon Sep 17 00:00:00 2001 From: Tim Bussmann Date: Thu, 1 Jul 2021 15:16:11 +0200 Subject: [PATCH 13/42] add acceptance test --- .../Recoverability/When_retry_is_confirmed.cs | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs new file mode 100644 index 0000000000..b78dc72c65 --- /dev/null +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs @@ -0,0 +1,104 @@ +namespace ServiceControl.AcceptanceTests.Recoverability +{ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + using AcceptanceTesting; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NUnit.Framework; + using ServiceControl.MessageFailures; + using ServiceControl.MessageFailures.Api; + using TestSupport.EndpointTemplates; + + [RunOnAllTransports] + class When_retry_is_confirmed : AcceptanceTest + { + [Ignore("Needs to target NSB 7.5 to pass")] + [Test] + public async Task Should_mark_message_as_successfully_resolved() + { + var context = await Define() + .WithEndpoint(b => b + .DoNotFailOnErrorMessages() + .When(s => s.SendLocal(new RetryMessage()))) + .Do("Wait for failed message", async c => + { + // wait till the failed message has been ingested + var tryGetMany = await this.TryGetMany("/api/errors"); + return tryGetMany; + }) + .Do("Retry message", async c => + { + //trigger retry + await this.Post("/api/errors/retry/all"); + }) + .Do("Wait for retry confirmation", async c => { + if (!c.ReceivedRetry) + { + // wait till endpoint processed the retried message + return false; + } + + var failedMessages = await this.TryGetMany("/api/errors"); + if (failedMessages.Items.Any(i => i.Status == FailedMessageStatus.Resolved)) + { + c.MessagesView = failedMessages.Items; + return true; + } + + return false; + }) + .Done(c => true) + .Run(); + + Assert.AreEqual(1, context.MessagesView.Count); + var failedMessage = context.MessagesView.Single(); + Assert.AreEqual(FailedMessageStatus.Resolved, failedMessage.Status); + Assert.AreEqual(1, failedMessage.NumberOfProcessingAttempts); + } + + class Context : ScenarioContext, ISequenceContext + { + public bool ThrowOnHandler { get; set; } = true; + public bool ReceivedRetry { get; set; } + public int Step { get; set; } + public List MessagesView { get; set; } + } + + class RetryingEndpoint : EndpointConfigurationBuilder + { + public RetryingEndpoint() + { + EndpointSetup(c => c.NoRetries()); + } + + class RetryMessageHandler : IHandleMessages + { + Context testContext; + + public RetryMessageHandler(Context testContext) + { + this.testContext = testContext; + } + + public Task Handle(RetryMessage message, IMessageHandlerContext context) + { + if (testContext.ThrowOnHandler) + { + testContext.ThrowOnHandler = false; + throw new Exception("boom"); + } + + testContext.ReceivedRetry = true; + return Task.CompletedTask; + } + } + } + + class RetryMessage : IMessage + { + } + } +} \ No newline at end of file From 731b57df2d883c6af924caaec50aa4755c9de39e Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Fri, 2 Jul 2021 12:05:55 +0200 Subject: [PATCH 14/42] Use ack-to header instead of version header --- .../When_a_retry_fails_to_be_sent.cs | 5 ++-- .../Recoverability/When_retry_is_confirmed.cs | 3 ++- .../Auditing/AuditEnricherContext.cs | 12 +++++++-- .../Auditing/AuditIngestor.cs | 2 +- .../Auditing/AuditPersister.cs | 27 ++++++++++++------- .../Recoverability/FailedMessagesFeature.cs | 23 +++++++++++++--- .../Recoverability/Retry_State_Tests.cs | 11 ++++---- .../ReturnToSenderDequeuerTests.cs | 13 ++++----- .../Retrying/Infrastructure/ReturnToSender.cs | 9 ++++--- 9 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs index fea74220df..a45109d3cc 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs @@ -13,6 +13,7 @@ using NUnit.Framework; using Operations.BodyStorage; using Raven.Client; + using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.MessageFailures; using ServiceControl.MessageFailures.Api; using ServiceControl.Recoverability; @@ -26,7 +27,7 @@ public async Task SubsequentBatchesShouldBeProcessed() { FailedMessage decomissionedFailure = null, successfullyRetried = null; - CustomConfiguration = config => config.RegisterComponents(components => components.ConfigureComponent(b => new FakeReturnToSender(b.Build(), b.Build(), b.Build()), DependencyLifecycle.SingleInstance)); + CustomConfiguration = config => config.RegisterComponents(components => components.ConfigureComponent(b => new FakeReturnToSender(b.Build(), b.Build(), b.Build(), b.Build()), DependencyLifecycle.SingleInstance)); await Define() .WithEndpoint(b => b.DoNotFailOnErrorMessages() @@ -148,7 +149,7 @@ public class MessageThatWillFail : ICommand public class FakeReturnToSender : ReturnToSender { - public FakeReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, MyContext myContext) : base(bodyStorage, documentStore) + public FakeReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, Settings settings, MyContext myContext) : base(bodyStorage, documentStore, settings) { this.myContext = myContext; } diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs index b78dc72c65..966beba365 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs @@ -34,7 +34,8 @@ public async Task Should_mark_message_as_successfully_resolved() //trigger retry await this.Post("/api/errors/retry/all"); }) - .Do("Wait for retry confirmation", async c => { + .Do("Wait for retry confirmation", async c => + { if (!c.ReceivedRetry) { // wait till endpoint processed the retried message diff --git a/src/ServiceControl.Audit/Auditing/AuditEnricherContext.cs b/src/ServiceControl.Audit/Auditing/AuditEnricherContext.cs index 60afd760b8..78610e1b6f 100644 --- a/src/ServiceControl.Audit/Auditing/AuditEnricherContext.cs +++ b/src/ServiceControl.Audit/Auditing/AuditEnricherContext.cs @@ -2,13 +2,15 @@ { using System.Collections.Generic; using NServiceBus; + using NServiceBus.Transport; class AuditEnricherContext { - public AuditEnricherContext(IReadOnlyDictionary headers, IList outgoingCommands, IDictionary metadata) + public AuditEnricherContext(IReadOnlyDictionary headers, IList outgoingCommands, IList outgoingSends, IDictionary metadata) { Headers = headers; this.outgoingCommands = outgoingCommands; + this.outgoingSends = outgoingSends; Metadata = metadata; } @@ -21,6 +23,12 @@ public void AddForSend(ICommand command) outgoingCommands.Add(command); } - IList outgoingCommands; + public void AddForSend(TransportOperation transportOperation) + { + outgoingSends.Add(transportOperation); + } + + readonly IList outgoingCommands; + readonly IList outgoingSends; } } \ No newline at end of file diff --git a/src/ServiceControl.Audit/Auditing/AuditIngestor.cs b/src/ServiceControl.Audit/Auditing/AuditIngestor.cs index 8db5b9b29c..e133381b19 100644 --- a/src/ServiceControl.Audit/Auditing/AuditIngestor.cs +++ b/src/ServiceControl.Audit/Auditing/AuditIngestor.cs @@ -24,7 +24,7 @@ public async Task Ingest(List contexts) log.Debug($"Ingesting {contexts.Count} message contexts"); } - var stored = await auditPersister.Persist(contexts).ConfigureAwait(false); + var stored = await auditPersister.Persist(contexts, dispatcher).ConfigureAwait(false); try { diff --git a/src/ServiceControl.Audit/Auditing/AuditPersister.cs b/src/ServiceControl.Audit/Auditing/AuditPersister.cs index cded9c91d8..84d2c92c38 100644 --- a/src/ServiceControl.Audit/Auditing/AuditPersister.cs +++ b/src/ServiceControl.Audit/Auditing/AuditPersister.cs @@ -11,6 +11,7 @@ using Monitoring; using Newtonsoft.Json; using NServiceBus; + using NServiceBus.Extensibility; using NServiceBus.Logging; using NServiceBus.Transport; using Raven.Abstractions.Data; @@ -41,7 +42,7 @@ public void Initialize(IMessageSession messageSession) this.messageSession = messageSession; } - public async Task> Persist(List contexts) + public async Task> Persist(List contexts, IDispatchMessages dispatcher) { var stopwatch = Stopwatch.StartNew(); @@ -64,7 +65,7 @@ public async Task> Persist(List co var inserts = new List(contexts.Count); foreach (var context in contexts) { - inserts.Add(ProcessMessage(context)); + inserts.Add(ProcessMessage(context, dispatcher)); } await Task.WhenAll(inserts).ConfigureAwait(false); @@ -87,7 +88,7 @@ public async Task> Persist(List co if (Logger.IsDebugEnabled) { - Logger.Debug($"Adding audit message for bulk storage"); + Logger.Debug("Adding audit message for bulk storage"); } using (auditBulkInsertDurationMeter.Measure()) @@ -196,7 +197,7 @@ static void RecordKnownEndpoints(EndpointDetails observedEndpoint, Dictionary knownEndpoint.LastSeen ? processedMessage.ProcessedAt : knownEndpoint.LastSeen; } - async Task ProcessMessage(MessageContext context) + async Task ProcessMessage(MessageContext context, IDispatchMessages dispatcher) { if (context.Headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageType) && messageType == typeof(SagaUpdatedMessage).FullName) @@ -205,7 +206,7 @@ async Task ProcessMessage(MessageContext context) } else { - await ProcessAuditMessage(context).ConfigureAwait(false); + await ProcessAuditMessage(context, dispatcher).ConfigureAwait(false); } } @@ -237,7 +238,7 @@ void ProcessSagaAuditMessage(MessageContext context) } } - async Task ProcessAuditMessage(MessageContext context) + async Task ProcessAuditMessage(MessageContext context, IDispatchMessages dispatcher) { if (!context.Headers.TryGetValue(Headers.MessageId, out var messageId)) { @@ -253,7 +254,8 @@ async Task ProcessAuditMessage(MessageContext context) }; var commandsToEmit = new List(); - var enricherContext = new AuditEnricherContext(context.Headers, commandsToEmit, metadata); + var messagesToEmit = new List(); + var enricherContext = new AuditEnricherContext(context.Headers, commandsToEmit, messagesToEmit, metadata); foreach (var enricher in enrichers) { @@ -277,16 +279,21 @@ await bodyStorageEnricher.StoreAuditMessageBody(context.Body, auditMessage) if (Logger.IsDebugEnabled) { - Logger.Debug($"Emitting {commandsToEmit.Count} commands"); + Logger.Debug($"Emitting {commandsToEmit.Count} commands and {messagesToEmit.Count} control messages."); } foreach (var commandToEmit in commandsToEmit) { await messageSession.Send(commandToEmit) .ConfigureAwait(false); } + + await dispatcher.Dispatch(new TransportOperations(messagesToEmit.ToArray()), + new TransportTransaction(), //Do not hook into the incoming transaction + new ContextBag()).ConfigureAwait(false); + if (Logger.IsDebugEnabled) { - Logger.Debug($"{commandsToEmit.Count} commands emitted."); + Logger.Debug($"{commandsToEmit.Count} commands and {messagesToEmit.Count} control messages emitted."); } if (metadata.TryGetValue("SendingEndpoint", out var sendingEndpoint)) @@ -324,6 +331,6 @@ await messageSession.Send(commandToEmit) readonly IDocumentStore store; readonly BodyStorageFeature.BodyStorageEnricher bodyStorageEnricher; IMessageSession messageSession; - static ILog Logger = LogManager.GetLogger(); + static readonly ILog Logger = LogManager.GetLogger(); } } diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index d657596090..4aee1a02c1 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -8,6 +8,8 @@ using Infrastructure; using NServiceBus; using NServiceBus.Features; + using NServiceBus.Routing; + using NServiceBus.Transport; class FailedMessagesFeature : Feature { @@ -29,6 +31,7 @@ public void Enrich(AuditEnricherContext context) var isOldRetry = headers.TryGetValue("ServiceControl.RetryId", out _); var isNewRetry = headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var newRetryMessageId); var isAckHandled = headers.ContainsKey("ServiceControl.Retry.AcknowledgementSent"); + var hasAckQueue = headers.TryGetValue("ServiceControl.Retry.AcknowledgementQueue", out var ackQueue); var hasBeenRetried = isOldRetry || isNewRetry; @@ -39,11 +42,23 @@ public void Enrich(AuditEnricherContext context) return; } - context.AddForSend(new MarkMessageFailureResolvedByRetry + if (hasAckQueue && isNewRetry) { - FailedMessageId = isOldRetry ? headers.UniqueId() : newRetryMessageId, - AlternativeFailedMessageIds = GetAlternativeUniqueMessageId(headers).ToArray() - }); + var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary + { + ["ServiceControl.Retry.Successful"] = "true" + }, new byte[0]); + var ackOperation = new TransportOperation(ackMessage, new UnicastAddressTag(ackQueue)); + context.AddForSend(ackOperation); + } + else + { + context.AddForSend(new MarkMessageFailureResolvedByRetry + { + FailedMessageId = isOldRetry ? headers.UniqueId() : newRetryMessageId, + AlternativeFailedMessageIds = GetAlternativeUniqueMessageId(headers).ToArray() + }); + } } IEnumerable GetAlternativeUniqueMessageId(IReadOnlyDictionary headers) diff --git a/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs b/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs index 1879b4eb4d..427f1c456d 100644 --- a/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs +++ b/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs @@ -13,6 +13,7 @@ using NUnit.Framework; using Operations; using Raven.Client; + using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.Infrastructure.BackgroundTasks; using ServiceControl.Infrastructure.DomainEvents; using ServiceControl.Operations.BodyStorage.RavenAttachments; @@ -77,7 +78,7 @@ public async Task When_a_group_is_prepared_with_three_batches_and_SC_is_restarte var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"), retryManager); + var processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"), retryManager); documentStore.WaitForIndexing(); @@ -94,7 +95,7 @@ public async Task When_a_group_is_prepared_with_three_batches_and_SC_is_restarte await documentManager.RebuildRetryOperationState(session); - processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"), retryManager); + processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"), retryManager); await processor.ProcessBatches(session, sender); await session.SaveChangesAsync(); @@ -119,7 +120,7 @@ public async Task When_a_group_is_forwarded_the_status_is_Completed() var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"); + var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"); var processor = new RetryProcessor(documentStore, domainEvents, returnToSender, retryManager); using (var session = documentStore.OpenAsyncSession()) @@ -160,7 +161,7 @@ public async Task When_there_is_one_poison_message_it_is_removed_from_batch_and_ var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"); + var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"); var processor = new RetryProcessor(documentStore, domainEvents, returnToSender, retryManager); bool c; @@ -203,7 +204,7 @@ public async Task When_a_group_has_one_batch_out_of_two_forwarded_the_status_is_ var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new ReturnToSender(bodyStorage, documentStore); + var returnToSender = new ReturnToSender(bodyStorage, documentStore, new Settings()); var sender = new TestSender(); diff --git a/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs b/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs index 07ffadd288..63b5944ada 100644 --- a/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs +++ b/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs @@ -11,6 +11,7 @@ using NServiceBus.Extensibility; using NServiceBus.Transport; using NUnit.Framework; + using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.CompositeViews.Messages; using ServiceControl.Operations.BodyStorage; using ServiceControl.Recoverability; @@ -42,7 +43,7 @@ public async Task It_removes_staging_id_header() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); Assert.IsFalse(sender.Message.Headers.ContainsKey("ServiceControl.Retry.StagingId")); @@ -61,7 +62,7 @@ public async Task It_fetches_the_body_from_storage_if_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); Assert.AreEqual("MessageBodyId", Encoding.UTF8.GetString(sender.Message.Body)); @@ -110,7 +111,7 @@ await session.StoreAsync(new FailedMessage documentStore.WaitForIndexing(); - await new ReturnToSender(null, documentStore).HandleMessage(message, sender) + await new ReturnToSender(null, documentStore, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); Assert.AreEqual("MessageBodyId", Encoding.UTF8.GetString(sender.Message.Body)); @@ -130,7 +131,7 @@ public async Task It_uses_retry_to_if_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); Assert.AreEqual("Proxy", sender.Destination); @@ -149,7 +150,7 @@ public async Task It_sends_directly_to_target_if_retry_to_is_not_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); Assert.AreEqual("TargetEndpoint", sender.Destination); @@ -171,7 +172,7 @@ public async Task It_restores_body_id_and_target_addres_after_failure() try { - await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) .ConfigureAwait(false); } catch (Exception) diff --git a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs index de0fe468c7..ab28728b29 100644 --- a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs +++ b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs @@ -13,12 +13,14 @@ namespace ServiceControl.Recoverability using Operations.BodyStorage; using Raven.Client; using Raven.Json.Linq; + using ServiceBus.Management.Infrastructure.Settings; class ReturnToSender { - public ReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore) + public ReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, Settings settings) { this.documentStore = documentStore; + this.settings = settings; this.bodyStorage = bodyStorage; } @@ -27,7 +29,7 @@ public virtual async Task HandleMessage(MessageContext message, IDispatchMessage var outgoingHeaders = new Dictionary(message.Headers); outgoingHeaders.Remove("ServiceControl.Retry.StagingId"); - outgoingHeaders["ServiceControl.Version"] = GitVersionInformation.MajorMinorPatch; + outgoingHeaders["ServiceControl.Retry.AcknowledgementQueue"] = settings.ErrorQueue; byte[] body = null; var messageId = message.MessageId; @@ -146,7 +148,8 @@ async Task FetchFromBodyStore(string attemptMessageId, string messageId) static readonly byte[] EmptyBody = new byte[0]; readonly IBodyStorage bodyStorage; - static ILog Log = LogManager.GetLogger(typeof(ReturnToSender)); + static readonly ILog Log = LogManager.GetLogger(typeof(ReturnToSender)); readonly IDocumentStore documentStore; + readonly Settings settings; } } From 686cd88552f10319310f738395102badf9a1f1d2 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Fri, 2 Jul 2021 12:24:23 +0200 Subject: [PATCH 15/42] Add test for legacy SC retry ack path --- .../Recoverability/FailedMessagesFeature.cs | 3 +- ...t_from_old_sc_is_sent_to_audit_instance.cs | 125 ++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/When_a_message_retry_audit_from_old_sc_is_sent_to_audit_instance.cs diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index 4aee1a02c1..6565db8ec8 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -46,7 +46,8 @@ public void Enrich(AuditEnricherContext context) { var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary { - ["ServiceControl.Retry.Successful"] = "true" + ["ServiceControl.Retry.Successful"] = "true", + ["ServiceControl.Retry.UniqueMessageId"] = newRetryMessageId }, new byte[0]); var ackOperation = new TransportOperation(ackMessage, new UnicastAddressTag(ackQueue)); context.AddForSend(ackOperation); diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/When_a_message_retry_audit_from_old_sc_is_sent_to_audit_instance.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/When_a_message_retry_audit_from_old_sc_is_sent_to_audit_instance.cs new file mode 100644 index 0000000000..661383b3d6 --- /dev/null +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/When_a_message_retry_audit_from_old_sc_is_sent_to_audit_instance.cs @@ -0,0 +1,125 @@ +namespace ServiceControl.MultiInstance.AcceptanceTests.Recoverability +{ + using System; + using System.Threading.Tasks; + using AcceptanceTesting; + using MessageFailures; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NServiceBus.Pipeline; + using NServiceBus.Settings; + using NUnit.Framework; + using ServiceControl.Infrastructure; + using TestSupport; + using TestSupport.EndpointTemplates; + + [RunOnAllTransports] + class When_a_message_retry_audit_from_old_sc_is_sent_to_audit_instance : AcceptanceTest + { + [Test] + public async Task Should_mark_as_resolved() + { + FailedMessage failure; + + await Define() + .WithEndpoint(b => b.When(session => session.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) + .Done(async c => + { + var result = await GetFailedMessage(c); + failure = result; + if (!result) + { + return false; + } + + if (failure.Status == FailedMessageStatus.Unresolved) + { + await IssueRetry(c, () => this.Post($"/api/errors/{failure.UniqueMessageId}/retry", null, null, ServiceControlInstanceName)); + return false; + } + + return failure.Status == FailedMessageStatus.Resolved; + }) + .Run(TimeSpan.FromMinutes(2)); + } + + Task> GetFailedMessage(MyContext c) + { + if (c.MessageId == null) + { + return Task.FromResult(SingleResult.Empty); + } + + return this.TryGet("/api/errors/" + c.UniqueMessageId, msg => true, ServiceControlInstanceName); + } + + async Task IssueRetry(MyContext c, Func retryAction) + { + if (!c.RetryIssued) + { + c.RetryIssued = true; + await retryAction(); + } + } + + public class Failing : EndpointConfigurationBuilder + { + public Failing() + { + EndpointSetup(c => + { + c.NoRetries(); + c.Pipeline.Register(new SimulateOldServiceControlBehavior(), "Simulates old SC behavior"); + }); + } + + public class SimulateOldServiceControlBehavior : Behavior + { + public override Task Invoke(ITransportReceiveContext context, Func next) + { + context.Message.Headers.Remove("ServiceControl.Retry.AcknowledgementQueue"); + return next(); + } + } + + public class MyMessageHandler : IHandleMessages + { + public MyContext Context { get; set; } + + public ReadOnlySettings Settings { get; set; } + + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + Console.Out.WriteLine("Handling message"); + Context.EndpointNameOfReceivingEndpoint = Settings.EndpointName(); + Context.LocalAddress = Settings.LocalAddress(); + Context.MessageId = context.MessageId.Replace(@"\", "-"); + + if (!Context.RetryIssued) //simulate that the exception will be resolved with the retry + { + Console.Out.WriteLine("Throwing exception for MyMessage"); + throw new Exception("Simulated exception"); + } + + return Task.FromResult(0); + } + } + } + + + public class MyMessage : ICommand + { + } + + public class MyContext : ScenarioContext + { + public string MessageId { get; set; } + + public string EndpointNameOfReceivingEndpoint { get; set; } + + public string UniqueMessageId => DeterministicGuid.MakeId(MessageId, EndpointNameOfReceivingEndpoint).ToString(); + public string LocalAddress { get; set; } + public bool RetryIssued { get; set; } + } + } +} \ No newline at end of file From 8a6bce31b9d08d0002866c2e7066a8573155ca70 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 5 Jul 2021 14:27:46 +0200 Subject: [PATCH 16/42] Add descriptive successful retry handling comments --- .../Recoverability/FailedMessagesFeature.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index 6565db8ec8..38430e6aeb 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -39,11 +39,15 @@ public void Enrich(AuditEnricherContext context) if (!hasBeenRetried || isAckHandled) { + //The message has not been sent for retry from ServiceControl or the endpoint indicated that is already has sent a retry acknowledgement to the + //ServiceControl main instance. Nothing to do. return; } if (hasAckQueue && isNewRetry) { + //The message has been sent for retry from ServiceControl 4.20 or higher (has the ACK queue header) but the endpoint did not recognized the header + //and did not sent the acknowledgement. We send it here to the error queue of the main instance. var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary { ["ServiceControl.Retry.Successful"] = "true", @@ -54,6 +58,8 @@ public void Enrich(AuditEnricherContext context) } else { + //The message has been sent for retry from ServiceControl older than 4.20. Regardless which version the endpoint was, we need to send a legacy confirmation + //message because the main instance of ServiceControl may still be on version lower than 4.19. context.AddForSend(new MarkMessageFailureResolvedByRetry { FailedMessageId = isOldRetry ? headers.UniqueId() : newRetryMessageId, From 53576cee7684cbdfc38f3db2db29cd015bb3d04f Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 5 Jul 2021 14:28:21 +0200 Subject: [PATCH 17/42] Add more descriptive comments for the legacy successful retry confirmation messages --- .../MessageFailures/MarkMessageFailureResolvedByRetry.cs | 5 +++++ .../MessageFailures/MessageFailureResolvedByRetry.cs | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl/Contracts/MessageFailures/MarkMessageFailureResolvedByRetry.cs b/src/ServiceControl/Contracts/MessageFailures/MarkMessageFailureResolvedByRetry.cs index 3e9a8111c2..201a9abad3 100644 --- a/src/ServiceControl/Contracts/MessageFailures/MarkMessageFailureResolvedByRetry.cs +++ b/src/ServiceControl/Contracts/MessageFailures/MarkMessageFailureResolvedByRetry.cs @@ -2,6 +2,11 @@ { using NServiceBus; + /// + /// A command used before ServiceControl 4.20. Sent by the Audit instance to the Main instance when the Audit instances detected that the audited message has the ServiceControl. + /// When upgrading from 4.x to 4.20 and higher these legacy messages may still be in the input queue so a handler for these is needed. + /// retry header. + /// public class MarkMessageFailureResolvedByRetry : ICommand { public string FailedMessageId { get; set; } diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs index eceb3888f3..c33bef395e 100644 --- a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs @@ -2,7 +2,10 @@ { using NServiceBus; - // Comes from unconverted legacy instances + /// + /// A message used by ServiceControl before Audit split (before 4.0) to inform that the audited message has been successfully retried. When upgrading from 3.x to 4.20 and higher + /// these legacy messages may still be in the input queue so a handler for these is needed. + /// public class MessageFailureResolvedByRetry : IMessage { public string FailedMessageId { get; set; } From 7a2b18b40219d93fde36020221ecbaf0a7bdf0d2 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 5 Jul 2021 14:28:57 +0200 Subject: [PATCH 18/42] Split handler for mark resolve messages into regular and legacy --- .../LegacyMessageFailureResolvedHandler.cs | 101 ++++++++++++++++++ .../Handlers/MessageFailureResolvedHandler.cs | 62 +---------- 2 files changed, 104 insertions(+), 59 deletions(-) create mode 100644 src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs diff --git a/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs b/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs new file mode 100644 index 0000000000..cc536c6d7b --- /dev/null +++ b/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs @@ -0,0 +1,101 @@ +namespace ServiceControl.MessageFailures.Handlers +{ + using System; + using System.Linq; + using System.Threading.Tasks; + using Contracts.MessageFailures; + using Infrastructure.DomainEvents; + using NServiceBus; + using Raven.Client; + using Recoverability; + + /// + /// This class handles legacy messages that mark a failed message as successfully retried. For further details go to message definitions. + /// + class LegacyMessageFailureResolvedHandler : + IHandleMessages, + IHandleMessages + { + public LegacyMessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainEvents, RetryDocumentManager retryDocumentManager) + { + this.store = store; + this.domainEvents = domainEvents; + this.retryDocumentManager = retryDocumentManager; + } + + public async Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHandlerContext context) + { + await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) + .ConfigureAwait(false); + await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + { + AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, + FailedMessageId = message.FailedMessageId + }).ConfigureAwait(false); + } + + // This is only needed because we might get this from legacy not yet converted instances + public async Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerContext context) + { + await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) + .ConfigureAwait(false); + await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + { + AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, + FailedMessageId = message.FailedMessageId + }).ConfigureAwait(false); + } + + async Task MarkAsResolvedByRetry(string primaryId, string[] messageAlternativeFailedMessageIds) + { + await retryDocumentManager.RemoveFailedMessageRetryDocument(primaryId).ConfigureAwait(false); + if (await MarkMessageAsResolved(primaryId) + .ConfigureAwait(false)) + { + return; + } + + if (messageAlternativeFailedMessageIds == null) + { + return; + } + + foreach (var alternative in messageAlternativeFailedMessageIds.Where(x => x != primaryId)) + { + await retryDocumentManager.RemoveFailedMessageRetryDocument(alternative).ConfigureAwait(false); + if (await MarkMessageAsResolved(alternative) + .ConfigureAwait(false)) + { + return; + } + } + } + + async Task MarkMessageAsResolved(string failedMessageId) + { + + using (var session = store.OpenAsyncSession()) + { + session.Advanced.UseOptimisticConcurrency = true; + + var failedMessage = await session.LoadAsync(new Guid(failedMessageId)) + .ConfigureAwait(false); + + if (failedMessage == null) + { + return false; + } + + failedMessage.Status = FailedMessageStatus.Resolved; + + await session.SaveChangesAsync().ConfigureAwait(false); + + return true; + } + } + + IDocumentStore store; + IDomainEvents domainEvents; + RetryDocumentManager retryDocumentManager; + } +} \ No newline at end of file diff --git a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs index 48b6d8edfe..03cd6fe7bd 100644 --- a/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs +++ b/src/ServiceControl/MessageFailures/Handlers/MessageFailureResolvedHandler.cs @@ -1,7 +1,6 @@ namespace ServiceControl.MessageFailures.Handlers { using System; - using System.Linq; using System.Threading.Tasks; using Api; using Contracts.MessageFailures; @@ -9,67 +8,15 @@ using InternalMessages; using NServiceBus; using Raven.Client; - using Recoverability; class MessageFailureResolvedHandler : - IHandleMessages, - IHandleMessages, IHandleMessages, IHandleMessages { - public MessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainEvents, RetryDocumentManager retryDocumentManager) + public MessageFailureResolvedHandler(IDocumentStore store, IDomainEvents domainEvents) { this.store = store; this.domainEvents = domainEvents; - this.retryDocumentManager = retryDocumentManager; - } - - public async Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHandlerContext context) - { - await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) - .ConfigureAwait(false); - await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent - { - AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, - FailedMessageId = message.FailedMessageId - }).ConfigureAwait(false); - } - - // This is only needed because we might get this from legacy not yet converted instances - public async Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerContext context) - { - await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) - .ConfigureAwait(false); - await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent - { - AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, - FailedMessageId = message.FailedMessageId - }).ConfigureAwait(false); - } - - async Task MarkAsResolvedByRetry(string primaryId, string[] messageAlternativeFailedMessageIds) - { - await retryDocumentManager.RemoveFailedMessageRetryDocument(primaryId).ConfigureAwait(false); - if (await MarkMessageAsResolved(primaryId) - .ConfigureAwait(false)) - { - return; - } - - if (messageAlternativeFailedMessageIds == null) - { - return; - } - - foreach (var alternative in messageAlternativeFailedMessageIds.Where(x => x != primaryId)) - { - await retryDocumentManager.RemoveFailedMessageRetryDocument(alternative).ConfigureAwait(false); - if (await MarkMessageAsResolved(alternative) - .ConfigureAwait(false)) - { - return; - } - } } public async Task Handle(MarkPendingRetriesAsResolved message, IMessageHandlerContext context) @@ -114,7 +61,7 @@ await domainEvents.Raise(new MessageFailureResolvedManually }).ConfigureAwait(false); } - async Task MarkMessageAsResolved(string failedMessageId) + async Task MarkMessageAsResolved(string failedMessageId) { using (var session = store.OpenAsyncSession()) @@ -126,19 +73,16 @@ async Task MarkMessageAsResolved(string failedMessageId) if (failedMessage == null) { - return false; + return; } failedMessage.Status = FailedMessageStatus.Resolved; await session.SaveChangesAsync().ConfigureAwait(false); - - return true; } } IDocumentStore store; IDomainEvents domainEvents; - RetryDocumentManager retryDocumentManager; } } \ No newline at end of file From 45946fe5974747bcb13a2a59579abe7d91bdf0cd Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 6 Jul 2021 11:11:23 +0200 Subject: [PATCH 19/42] Review suggestions --- .../Messages/ScatterGatherApiMessageView.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs index 17a700ef3d..4c81773894 100644 --- a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs +++ b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs @@ -30,14 +30,13 @@ protected override IList ProcessResults(HttpRequestMessage request { result.BodyUrl += $"?instance_id={queryResult.InstanceId}"; } - } - //HINT: De-duplicate the results as some messages might be present in multiple instances (e.g. when they initially failed and later were successfully processed) - foreach (MessagesView messagesView in messagesViews) - { - if (!deduplicated.ContainsKey(messagesView.MessageId)) + //HINT: De-duplicate the results as some messages might be present in multiple instances (e.g. when they initially failed and later were successfully processed) + //The Execute method guarantees that the first item in the results collection comes from the main SC instance so the data fetched from failed messages has + //precedence over the data from the audit instances. + if (!deduplicated.ContainsKey(result.MessageId)) { - deduplicated.Add(messagesView.Id, messagesView); + deduplicated.Add(result.Id, result); } } } From 869f36458324f4a9541c5f9f2a3160953d26fdc8 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 6 Jul 2021 14:44:59 +0200 Subject: [PATCH 20/42] Use date for the successful retry header --- .../Recoverability/FailedMessagesFeature.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index 38430e6aeb..b82814430a 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -50,7 +50,7 @@ public void Enrich(AuditEnricherContext context) //and did not sent the acknowledgement. We send it here to the error queue of the main instance. var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary { - ["ServiceControl.Retry.Successful"] = "true", + ["ServiceControl.Retry.Successful"] = DateTimeExtensions.ToWireFormattedString(DateTime.UtcNow), ["ServiceControl.Retry.UniqueMessageId"] = newRetryMessageId }, new byte[0]); var ackOperation = new TransportOperation(ackMessage, new UnicastAddressTag(ackQueue)); From 36682d46ca0398cd2626b029dcf04218774d392b Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Tue, 6 Jul 2021 14:48:03 +0200 Subject: [PATCH 21/42] MessageFailureResolvedByRetry legacy part moved to partial --- .../MessageFailureResolvedByRetry.Legacy.cs | 12 ++++++++++++ .../MessageFailures/MessageFailureResolvedByRetry.cs | 9 +++------ .../MessageFailureResolvedByRetryDomainEvent.cs | 11 ----------- .../MessageFailureResolvedByRetryDefinition.cs | 2 +- .../Handlers/LegacyMessageFailureResolvedHandler.cs | 4 ++-- .../Operations/RetryConfirmationProcessor.cs | 2 +- .../MessageFailureResolvedByRetryPublisher.cs | 4 ++-- 7 files changed, 21 insertions(+), 23 deletions(-) create mode 100644 src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.Legacy.cs delete mode 100644 src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.Legacy.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.Legacy.cs new file mode 100644 index 0000000000..5b36771d4e --- /dev/null +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.Legacy.cs @@ -0,0 +1,12 @@ +namespace ServiceControl.Contracts.MessageFailures +{ + using NServiceBus; + + /// + /// A message used by ServiceControl before Audit split (before 4.0) to inform that the audited message has been successfully retried. When upgrading from 3.x to 4.20 and higher + /// these legacy messages may still be in the input queue so a handler for these is needed. + /// + public partial class MessageFailureResolvedByRetry : IMessage + { + } +} \ No newline at end of file diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs index c33bef395e..346af32719 100644 --- a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs +++ b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetry.cs @@ -1,12 +1,9 @@ namespace ServiceControl.Contracts.MessageFailures { - using NServiceBus; + using Infrastructure.DomainEvents; + using Infrastructure.SignalR; - /// - /// A message used by ServiceControl before Audit split (before 4.0) to inform that the audited message has been successfully retried. When upgrading from 3.x to 4.20 and higher - /// these legacy messages may still be in the input queue so a handler for these is needed. - /// - public class MessageFailureResolvedByRetry : IMessage + public partial class MessageFailureResolvedByRetry : IDomainEvent, IUserInterfaceEvent { public string FailedMessageId { get; set; } public string[] AlternativeFailedMessageIds { get; set; } diff --git a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs b/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs deleted file mode 100644 index 34640e8fcf..0000000000 --- a/src/ServiceControl/Contracts/MessageFailures/MessageFailureResolvedByRetryDomainEvent.cs +++ /dev/null @@ -1,11 +0,0 @@ -namespace ServiceControl.Contracts.MessageFailures -{ - using Infrastructure.DomainEvents; - using Infrastructure.SignalR; - - public class MessageFailureResolvedByRetryDomainEvent : IDomainEvent, IUserInterfaceEvent - { - public string FailedMessageId { get; set; } - public string[] AlternativeFailedMessageIds { get; set; } - } -} \ No newline at end of file diff --git a/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs b/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs index 93d3125303..23f31de096 100644 --- a/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs +++ b/src/ServiceControl/EventLog/Definitions/MessageFailureResolvedByRetryDefinition.cs @@ -2,7 +2,7 @@ { using Contracts.MessageFailures; - class MessageFailureResolvedByRetryDefinition : EventLogMappingDefinition + class MessageFailureResolvedByRetryDefinition : EventLogMappingDefinition { public MessageFailureResolvedByRetryDefinition() { diff --git a/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs b/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs index cc536c6d7b..348e8710c4 100644 --- a/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs +++ b/src/ServiceControl/MessageFailures/Handlers/LegacyMessageFailureResolvedHandler.cs @@ -27,7 +27,7 @@ public async Task Handle(MarkMessageFailureResolvedByRetry message, IMessageHand { await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) .ConfigureAwait(false); - await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + await domainEvents.Raise(new MessageFailureResolvedByRetry { AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, FailedMessageId = message.FailedMessageId @@ -39,7 +39,7 @@ public async Task Handle(MessageFailureResolvedByRetry message, IMessageHandlerC { await MarkAsResolvedByRetry(message.FailedMessageId, message.AlternativeFailedMessageIds) .ConfigureAwait(false); - await domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + await domainEvents.Raise(new MessageFailureResolvedByRetry { AlternativeFailedMessageIds = message.AlternativeFailedMessageIds, FailedMessageId = message.FailedMessageId diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index 8bdc3b5965..e59b03649c 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -51,7 +51,7 @@ public RetryConfirmationProcessor(IDomainEvents domainEvents) public Task Announce(MessageContext messageContext) { - return domainEvents.Raise(new MessageFailureResolvedByRetryDomainEvent + return domainEvents.Raise(new MessageFailureResolvedByRetry { FailedMessageId = messageContext.Headers[RetryUniqueMessageIdHeader], }); diff --git a/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs b/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs index 5f219f8633..a9fd5a1476 100644 --- a/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs +++ b/src/ServiceControl/Recoverability/ExternalIntegration/MessageFailureResolvedByRetryPublisher.cs @@ -7,9 +7,9 @@ namespace ServiceControl.Recoverability.ExternalIntegration using ExternalIntegrations; using Raven.Client; - class MessageFailureResolvedByRetryPublisher : EventPublisher + class MessageFailureResolvedByRetryPublisher : EventPublisher { - protected override DispatchContext CreateDispatchRequest(MessageFailureResolvedByRetryDomainEvent @event) + protected override DispatchContext CreateDispatchRequest(MessageFailureResolvedByRetry @event) { return new DispatchContext { From 171773b528bd070402ff0eadf6574ef8420629b1 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Thu, 8 Jul 2021 10:48:47 +0200 Subject: [PATCH 22/42] Use message id for deduplication --- .../CompositeViews/Messages/ScatterGatherApiMessageView.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs index 4c81773894..c35cb2fa3a 100644 --- a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs +++ b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApiMessageView.cs @@ -36,7 +36,7 @@ protected override IList ProcessResults(HttpRequestMessage request //precedence over the data from the audit instances. if (!deduplicated.ContainsKey(result.MessageId)) { - deduplicated.Add(result.Id, result); + deduplicated.Add(result.MessageId, result); } } } From 8353d18240acf82981afbbe4b011d119b4259042 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Thu, 8 Jul 2021 14:33:34 +0200 Subject: [PATCH 23/42] Upgrade to NServiceBus 7.5 and unignore the retry acknowledgement acceptance test --- .../Recoverability/When_retry_is_confirmed.cs | 1 - src/ServiceControl.Audit/ServiceControl.Audit.csproj | 2 +- .../ServiceControl.Infrastructure.RavenDB.csproj | 2 +- .../ServiceControl.LoadTests.Messages.csproj | 2 +- .../ServiceControl.LoadTests.Reporter.csproj | 2 +- src/ServiceControl.Monitoring/ServiceControl.Monitoring.csproj | 2 +- src/ServiceControl.SagaAudit/ServiceControl.SagaAudit.csproj | 2 +- src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs | 1 - src/ServiceControl/ServiceControl.csproj | 2 +- 9 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs index 966beba365..f46b28c490 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/When_retry_is_confirmed.cs @@ -15,7 +15,6 @@ [RunOnAllTransports] class When_retry_is_confirmed : AcceptanceTest { - [Ignore("Needs to target NSB 7.5 to pass")] [Test] public async Task Should_mark_message_as_successfully_resolved() { diff --git a/src/ServiceControl.Audit/ServiceControl.Audit.csproj b/src/ServiceControl.Audit/ServiceControl.Audit.csproj index c4bc9a243c..d1540f7f2c 100644 --- a/src/ServiceControl.Audit/ServiceControl.Audit.csproj +++ b/src/ServiceControl.Audit/ServiceControl.Audit.csproj @@ -28,7 +28,7 @@ - + diff --git a/src/ServiceControl.Infrastructure.RavenDB/ServiceControl.Infrastructure.RavenDB.csproj b/src/ServiceControl.Infrastructure.RavenDB/ServiceControl.Infrastructure.RavenDB.csproj index 452b0aa6db..52ec7c63ba 100644 --- a/src/ServiceControl.Infrastructure.RavenDB/ServiceControl.Infrastructure.RavenDB.csproj +++ b/src/ServiceControl.Infrastructure.RavenDB/ServiceControl.Infrastructure.RavenDB.csproj @@ -6,7 +6,7 @@ - + \ No newline at end of file diff --git a/src/ServiceControl.LoadTests.Messages/ServiceControl.LoadTests.Messages.csproj b/src/ServiceControl.LoadTests.Messages/ServiceControl.LoadTests.Messages.csproj index 9e5010316c..f6822e122c 100644 --- a/src/ServiceControl.LoadTests.Messages/ServiceControl.LoadTests.Messages.csproj +++ b/src/ServiceControl.LoadTests.Messages/ServiceControl.LoadTests.Messages.csproj @@ -5,7 +5,7 @@ - + \ No newline at end of file diff --git a/src/ServiceControl.LoadTests.Reporter/ServiceControl.LoadTests.Reporter.csproj b/src/ServiceControl.LoadTests.Reporter/ServiceControl.LoadTests.Reporter.csproj index 3a456d934d..fd125286b8 100644 --- a/src/ServiceControl.LoadTests.Reporter/ServiceControl.LoadTests.Reporter.csproj +++ b/src/ServiceControl.LoadTests.Reporter/ServiceControl.LoadTests.Reporter.csproj @@ -6,7 +6,7 @@ - + diff --git a/src/ServiceControl.Monitoring/ServiceControl.Monitoring.csproj b/src/ServiceControl.Monitoring/ServiceControl.Monitoring.csproj index 9d640d24ca..29bd8d4827 100644 --- a/src/ServiceControl.Monitoring/ServiceControl.Monitoring.csproj +++ b/src/ServiceControl.Monitoring/ServiceControl.Monitoring.csproj @@ -24,7 +24,7 @@ - + diff --git a/src/ServiceControl.SagaAudit/ServiceControl.SagaAudit.csproj b/src/ServiceControl.SagaAudit/ServiceControl.SagaAudit.csproj index 4b9ccfcc0e..e2b04a1a9f 100644 --- a/src/ServiceControl.SagaAudit/ServiceControl.SagaAudit.csproj +++ b/src/ServiceControl.SagaAudit/ServiceControl.SagaAudit.csproj @@ -5,7 +5,7 @@ - + diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs index 05c132c052..1310bff608 100644 --- a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs +++ b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs @@ -8,7 +8,6 @@ namespace ServiceControl.CompositeViews.Messages using System.Net.Http; using System.Threading.Tasks; using System.Web.Http; - using Autofac; using Infrastructure.Settings; using Infrastructure.WebApi; using Newtonsoft.Json; diff --git a/src/ServiceControl/ServiceControl.csproj b/src/ServiceControl/ServiceControl.csproj index 9efd00e61a..a82cea728b 100644 --- a/src/ServiceControl/ServiceControl.csproj +++ b/src/ServiceControl/ServiceControl.csproj @@ -32,7 +32,7 @@ - + From c2d382d263d5790ad1858e63c61eccf5bdfdf293 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Fri, 9 Jul 2021 09:12:20 +0200 Subject: [PATCH 24/42] Apply suggestions from code review Co-authored-by: Tim Bussmann --- .../Recoverability/FailedMessagesFeature.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index b82814430a..afbc538768 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -46,8 +46,8 @@ public void Enrich(AuditEnricherContext context) if (hasAckQueue && isNewRetry) { - //The message has been sent for retry from ServiceControl 4.20 or higher (has the ACK queue header) but the endpoint did not recognized the header - //and did not sent the acknowledgement. We send it here to the error queue of the main instance. + //The message has been sent for retry from ServiceControl 4.20 or higher (has the ACK queue header) but the endpoint did not recognize the header + //and did not send the acknowledgment. We send it here to the acknowledgment queue. var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary { ["ServiceControl.Retry.Successful"] = DateTimeExtensions.ToWireFormattedString(DateTime.UtcNow), @@ -100,4 +100,4 @@ static string ExtractQueueNameForLegacyReasons(string address) } } } -} \ No newline at end of file +} From 2283fad8d0673017e0a7cf6e4e740dbce6221479 Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Fri, 9 Jul 2021 13:10:39 +0200 Subject: [PATCH 25/42] fix tests --- .../ScatterGather/MessageView_ScatterGatherTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.UnitTests/ScatterGather/MessageView_ScatterGatherTest.cs b/src/ServiceControl.UnitTests/ScatterGather/MessageView_ScatterGatherTest.cs index 2d8eb6fb93..3f14043460 100644 --- a/src/ServiceControl.UnitTests/ScatterGather/MessageView_ScatterGatherTest.cs +++ b/src/ServiceControl.UnitTests/ScatterGather/MessageView_ScatterGatherTest.cs @@ -49,7 +49,7 @@ protected IEnumerable LocalData() { for (var i = 0; i < 200; i++) { - yield return new MessagesView(); + yield return new MessagesView { MessageId = Guid.NewGuid().ToString() }; } } @@ -57,7 +57,7 @@ protected IEnumerable RemoteData() { for (var i = 0; i < 55; i++) { - yield return new MessagesView(); + yield return new MessagesView { MessageId = Guid.NewGuid().ToString() }; } } From be7c01661eb83d5a7ec6a275ab1db084baaea5f1 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Fri, 9 Jul 2021 15:50:58 +0200 Subject: [PATCH 26/42] Tests for successful retry confirmation logic in the audits --- ...ssful_retry_at_old_endpoint_is_detected.cs | 97 +++++++++++++++++++ ...uccessful_retry_from_old_SC_is_detected.cs | 58 +++++++++++ .../When_a_successful_retry_is_detected.cs | 63 ++++++++---- 3 files changed, 198 insertions(+), 20 deletions(-) create mode 100644 src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs create mode 100644 src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_from_old_SC_is_detected.cs diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs new file mode 100644 index 0000000000..17e742819b --- /dev/null +++ b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs @@ -0,0 +1,97 @@ +namespace ServiceControl.Audit.AcceptanceTests.Recoverability +{ + using System; + using System.Threading.Tasks; + using AcceptanceTesting; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NServiceBus.Features; + using NServiceBus.Pipeline; + using NUnit.Framework; + using TestSupport.EndpointTemplates; + using Conventions = NServiceBus.AcceptanceTesting.Customization.Conventions; + + [RunOnAllTransports] + class When_a_successful_retry_at_old_endpoint_is_detected : AcceptanceTest + { + [Test] + public async Task Should_sent_acknowledgement() + { + var failedMessageId = Guid.NewGuid().ToString(); + var context = await Define() + .WithEndpoint() + .WithEndpoint(b => b.When(s => + { + var options = new SendOptions(); + + options.SetHeader("ServiceControl.Retry.UniqueMessageId", failedMessageId); + options.SetHeader("ServiceControl.Retry.AcknowledgementQueue", Conventions.EndpointNamingConvention(typeof(AcknowledgementSpy))); + options.RouteToThisEndpoint(); + return s.Send(new MyMessage(), options); + })) + .Done(c => c.AcknowledgementSent) + .Run(); + + Assert.IsTrue(context.AcknowledgementSent); + } + + public class Context : ScenarioContext + { + public bool AcknowledgementSent { get; set; } + } + + public class AcknowledgementSpy : EndpointConfigurationBuilder + { + public AcknowledgementSpy() + { + EndpointSetup(cfg => + { + cfg.Pipeline.Register(b => new SpyBehavior(b.Build()), "Spy behavior"); + cfg.Pipeline.Remove("DiscardMessagesBehavior"); //Otherwise the confirmation generated by the audit instance will be discarded + }); + } + + public class SpyBehavior : Behavior + { + Context scenarioContext; + + public SpyBehavior(Context scenarioContext) + { + this.scenarioContext = scenarioContext; + } + + public override Task Invoke(ITransportReceiveContext context, Func next) + { + if (context.Message.Headers.ContainsKey("ServiceControl.Retry.Successful")) + { + scenarioContext.AcknowledgementSent = true; + } + return Task.CompletedTask; + } + } + } + + public class Receiver : EndpointConfigurationBuilder + { + public Receiver() + { + EndpointSetup(cfg => + { + cfg.DisableFeature(); + }); + } + + public class MyMessageHandler : IHandleMessages + { + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + return Task.FromResult(0); + } + } + } + + public class MyMessage : ICommand + { + } + } +} \ No newline at end of file diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_from_old_SC_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_from_old_SC_is_detected.cs new file mode 100644 index 0000000000..abfa242f56 --- /dev/null +++ b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_from_old_SC_is_detected.cs @@ -0,0 +1,58 @@ +namespace ServiceControl.Audit.AcceptanceTests.Recoverability +{ + using System; + using System.Linq; + using System.Threading.Tasks; + using AcceptanceTesting; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NUnit.Framework; + using TestSupport; + using TestSupport.EndpointTemplates; + + [RunOnAllTransports] + class When_a_successful_retry_from_old_SC_is_detected : AcceptanceTest + { + [Test] + public async Task Should_raise_integration_event() + { + var uniqueMessageIdHeaderName = "ServiceControl.Retry.UniqueMessageId"; + + var failedMessageId = Guid.NewGuid().ToString(); + var context = await Define() + .WithEndpoint(b => b.When(s => + { + var options = new SendOptions(); + + options.SetHeader(uniqueMessageIdHeaderName, failedMessageId); + options.RouteToThisEndpoint(); + return s.Send(new MyMessage(), options); + })) + .Done(c => c.SentMarkMessageFailureResolvedByRetriesCommands.Any()) + .Run(); + + var command = context.SentMarkMessageFailureResolvedByRetriesCommands.Single(); + Assert.AreEqual(failedMessageId, command.FailedMessageId); + } + + public class Receiver : EndpointConfigurationBuilder + { + public Receiver() + { + EndpointSetup(); + } + + public class MyMessageHandler : IHandleMessages + { + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + return Task.FromResult(0); + } + } + } + + public class MyMessage : ICommand + { + } + } +} \ No newline at end of file diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs index 7b8219c52b..335693ce5a 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs @@ -1,49 +1,72 @@ namespace ServiceControl.Audit.AcceptanceTests.Recoverability { using System; - using System.Linq; using System.Threading.Tasks; using AcceptanceTesting; - using Audit.Auditing.MessagesView; using NServiceBus; using NServiceBus.AcceptanceTesting; + using NServiceBus.Pipeline; using NUnit.Framework; - using TestSupport; using TestSupport.EndpointTemplates; + using Conventions = NServiceBus.AcceptanceTesting.Customization.Conventions; + [RunOnAllTransports] class When_a_successful_retry_is_detected : AcceptanceTest { [Test] - public async Task Should_raise_integration_event() + public async Task Should_sent_acknowledgement() { - var uniqueMessageIdHeaderName = "ServiceControl.Retry.UniqueMessageId"; - var failedMessageId = Guid.NewGuid().ToString(); - var context = await Define() + var context = await Define() .WithEndpoint(b => b.When(s => { var options = new SendOptions(); - options.SetHeader(uniqueMessageIdHeaderName, failedMessageId); + options.SetHeader("ServiceControl.Retry.UniqueMessageId", failedMessageId); + options.SetHeader("ServiceControl.Retry.AcknowledgementQueue", Conventions.EndpointNamingConvention(typeof(AcknowledgementSpy))); options.RouteToThisEndpoint(); return s.Send(new MyMessage(), options); })) - .Done(async c => + .WithEndpoint() + .Done(c => c.AcknowledgementSent) + .Run(); + + Assert.IsTrue(context.AcknowledgementSent); + } + + public class Context : ScenarioContext + { + public bool AcknowledgementSent { get; set; } + } + + public class AcknowledgementSpy : EndpointConfigurationBuilder + { + public AcknowledgementSpy() + { + EndpointSetup(cfg => { - var result = await this.TryGetSingle("/api/messages", m => - { - var storedMessageId = m.Headers.Select(kv => kv.Value.ToString()) - .FirstOrDefault(v => v == failedMessageId); + cfg.Pipeline.Register(b => new SpyBehavior(b.Build()), "Spy behavior"); + }); + } - return storedMessageId == failedMessageId; - }); + public class SpyBehavior : Behavior + { + Context scenarioContext; - return result.HasResult; - }) - .Run(); + public SpyBehavior(Context scenarioContext) + { + this.scenarioContext = scenarioContext; + } - var command = context.SentMarkMessageFailureResolvedByRetriesCommands.Single(); - Assert.AreEqual(failedMessageId, command.FailedMessageId); + public override Task Invoke(ITransportReceiveContext context, Func next) + { + if (context.Message.Headers.ContainsKey("ServiceControl.Retry.Successful")) + { + scenarioContext.AcknowledgementSent = true; + } + return Task.CompletedTask; + } + } } public class Receiver : EndpointConfigurationBuilder From b108919238a5053c51b888029a251445a2ad872a Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 12 Jul 2021 10:07:59 +0200 Subject: [PATCH 27/42] Remove per-message exception handling on the ACK processing path --- .../Operations/ErrorIngestor.cs | 12 ++++----- .../Operations/RetryConfirmationProcessor.cs | 25 +++---------------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/ServiceControl/Operations/ErrorIngestor.cs b/src/ServiceControl/Operations/ErrorIngestor.cs index e4a58832ef..245b8eb5ba 100644 --- a/src/ServiceControl/Operations/ErrorIngestor.cs +++ b/src/ServiceControl/Operations/ErrorIngestor.cs @@ -42,7 +42,7 @@ public async Task Ingest(List contexts) } - var (storedFailed, storedRetried) = await PersistFailedMessages(failedMessages, retriedMessages) + var storedFailed = await PersistFailedMessages(failedMessages, retriedMessages) .ConfigureAwait(false); try @@ -52,7 +52,7 @@ public async Task Ingest(List contexts) { announcerTasks.Add(errorProcessor.Announce(context)); } - foreach (var context in storedRetried) + foreach (var context in retriedMessages) { announcerTasks.Add(retryConfirmationProcessor.Announce(context)); } @@ -72,7 +72,7 @@ public async Task Ingest(List contexts) } } - foreach (var context in storedFailed.Concat(storedRetried)) + foreach (var context in storedFailed.Concat(retriedMessages)) { context.GetTaskCompletionSource().TrySetResult(true); } @@ -89,7 +89,7 @@ public async Task Ingest(List contexts) } } - async Task<(IReadOnlyList, IReadOnlyList)> PersistFailedMessages(List failedMessageContexts, List retriedMessageContexts) + async Task> PersistFailedMessages(List failedMessageContexts, List retriedMessageContexts) { var stopwatch = Stopwatch.StartNew(); @@ -101,7 +101,7 @@ public async Task Ingest(List contexts) try { var (storedFailedMessageContexts, storeCommands) = await errorProcessor.Process(failedMessageContexts).ConfigureAwait(false); - var (storedRetriedMessageContexts, markRetriedCommands) = retryConfirmationProcessor.Process(retriedMessageContexts); + var markRetriedCommands = retryConfirmationProcessor.Process(retriedMessageContexts); using (bulkInsertDurationMeter.Measure()) { @@ -111,7 +111,7 @@ await store.AsyncDatabaseCommands.BatchAsync(allCommands) .ConfigureAwait(false); } - return (storedFailedMessageContexts, storedRetriedMessageContexts); + return storedFailedMessageContexts; } catch (Exception e) { diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index e59b03649c..7a357bb272 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -1,12 +1,10 @@ namespace ServiceControl.Operations { - using System; using System.Collections.Generic; using System.Threading.Tasks; using Contracts.MessageFailures; using Infrastructure.DomainEvents; using MessageFailures; - using NServiceBus.Logging; using NServiceBus.Transport; using Raven.Abstractions.Commands; using Raven.Abstractions.Data; @@ -22,31 +20,17 @@ public RetryConfirmationProcessor(IDomainEvents domainEvents) this.domainEvents = domainEvents; } - public (IReadOnlyList, IReadOnlyCollection) Process(List contexts) + public IReadOnlyCollection Process(List contexts) { - var storedContexts = new List(contexts.Count); var allCommands = new List(contexts.Count); foreach (var context in contexts) { - try - { - var commands = CreateDatabaseCommands(context); - allCommands.AddRange(commands); - storedContexts.Add(context); - } - catch (Exception e) - { - if (Logger.IsDebugEnabled) - { - Logger.Debug($"Processing of message '{context.MessageId}' failed.", e); - } - - context.GetTaskCompletionSource().TrySetException(e); - } + var commands = CreateDatabaseCommands(context); + allCommands.AddRange(commands); } - return (storedContexts, allCommands); + return allCommands; } public Task Announce(MessageContext messageContext) @@ -77,7 +61,6 @@ static IEnumerable CreateDatabaseCommands(MessageContext context) yield return deleteCommand; } - static readonly ILog Logger = LogManager.GetLogger(); readonly IDomainEvents domainEvents; } } \ No newline at end of file From c7c6bffb6e7ffe67b51bf00ddb81446d81f85d5b Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Mon, 12 Jul 2021 14:21:20 +0200 Subject: [PATCH 28/42] Make tests happy --- .../MessageFailures/When_a_failed_message_is_pending_retry.cs | 2 +- .../When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs | 3 +++ .../When_a_pending_retry_is_resolved_by_selection.cs | 3 +++ .../MessageFailures/When_a_pending_retry_is_retried_again.cs | 3 +++ .../When_a_pending_retry_is_retried_by_queue_and_timeframe.cs | 3 +++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_failed_message_is_pending_retry.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_failed_message_is_pending_retry.cs index ff61c05582..095659bd7a 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_failed_message_is_pending_retry.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_failed_message_is_pending_retry.cs @@ -57,7 +57,7 @@ public FailingEndpoint() EndpointSetup(c => { c.EnableFeature(); - + c.DisableFeature(); var recoverability = c.Recoverability(); recoverability.Immediate(s => s.NumberOfRetries(1)); recoverability.Delayed(s => s.NumberOfRetries(0)); diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs index 4595675fbf..03b2f86487 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_queue_and_timeframe.cs @@ -7,6 +7,7 @@ using Infrastructure; using NServiceBus; using NServiceBus.AcceptanceTesting; + using NServiceBus.Features; using NServiceBus.Settings; using NUnit.Framework; using ServiceControl.MessageFailures; @@ -63,6 +64,8 @@ public Failing() { EndpointSetup(c => { + //Do not inform SC that the message has been already successfully handled + c.DisableFeature(); c.NoRetries(); c.NoOutbox(); }); diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_selection.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_selection.cs index 04ad74d8cb..1949b9adcb 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_selection.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_resolved_by_selection.cs @@ -7,6 +7,7 @@ using Infrastructure; using NServiceBus; using NServiceBus.AcceptanceTesting; + using NServiceBus.Features; using NServiceBus.Settings; using NUnit.Framework; using ServiceControl.MessageFailures; @@ -55,6 +56,8 @@ public FailingEndpoint() { EndpointSetup(c => { + //Do not inform SC that the message has been already successfully handled + c.DisableFeature(); c.NoRetries(); c.NoOutbox(); }); diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_again.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_again.cs index bc667d5304..f3c16fcad8 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_again.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_again.cs @@ -7,6 +7,7 @@ using Infrastructure; using NServiceBus; using NServiceBus.AcceptanceTesting; + using NServiceBus.Features; using NServiceBus.Settings; using NUnit.Framework; using ServiceControl.MessageFailures; @@ -51,6 +52,8 @@ public FailingEndpoint() { EndpointSetup(c => { + //Do not inform SC that the message has been already successfully handled + c.DisableFeature(); c.NoRetries(); c.NoOutbox(); }); diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_by_queue_and_timeframe.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_by_queue_and_timeframe.cs index 537e9349bf..678c308fe1 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_by_queue_and_timeframe.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_pending_retry_is_retried_by_queue_and_timeframe.cs @@ -7,6 +7,7 @@ using Infrastructure; using NServiceBus; using NServiceBus.AcceptanceTesting; + using NServiceBus.Features; using NServiceBus.Settings; using NUnit.Framework; using ServiceControl.MessageFailures; @@ -54,6 +55,8 @@ public Failing() { EndpointSetup(c => { + //Do not inform SC that the message has been already successfully handled + c.DisableFeature(); c.NoRetries(); c.NoOutbox(); }); From 70e7a02223bffc4aee59d8c8aef5578a019502de Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Tue, 13 Jul 2021 10:12:18 +0200 Subject: [PATCH 29/42] retry confirmation idempotency tests --- .../RetryConfirmationProcessorTests.cs | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs diff --git a/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs b/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs new file mode 100644 index 0000000000..ec14fc1b50 --- /dev/null +++ b/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs @@ -0,0 +1,97 @@ +namespace ServiceControl.UnitTests.Recoverability +{ + using System; + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + using Contracts.MessageFailures; + using MessageFailures.Handlers; + using NServiceBus.Extensibility; + using NServiceBus.Testing; + using NServiceBus.Transport; + using NUnit.Framework; + using Operations; + using Raven.Client; + using Raven.Tests.Helpers; + using MessageFailures; + using Raven.Abstractions.Commands; + using ServiceControl.Operations; + using ServiceControl.Recoverability; + + public class RetryConfirmationProcessorTests : RavenTestBase + { + IDocumentStore Store { get; set; } + RetryConfirmationProcessor Processor { get; set; } + LegacyMessageFailureResolvedHandler Handler { get; set; } + + [SetUp] + public async Task Setup() + { + Store = NewDocumentStore(); + + var domainEvents = new FakeDomainEvents(); + Processor = new RetryConfirmationProcessor(domainEvents); + + var retryDocumentManager = new RetryDocumentManager(new FakeApplicationLifetime(), Store, new RetryingManager(domainEvents)); + Handler = new LegacyMessageFailureResolvedHandler(Store, domainEvents, retryDocumentManager); + + using (var session = Store.OpenAsyncSession()) + { + var failedMessage = new FailedMessage + { + Id = FailedMessage.MakeDocumentId(MessageId), + Status = FailedMessageStatus.Unresolved + }; + + await session.StoreAsync(failedMessage); + await session.SaveChangesAsync(); + } + + var retryDocumentCommands = RetryDocumentManager.CreateFailedMessageRetryDocument(Guid.NewGuid().ToString(), MessageId); + await Store.AsyncDatabaseCommands.BatchAsync(new[] { retryDocumentCommands }); + } + + [Test] + public void Should_handle_multiple_retry_confirmations_in_the_error_ingestion() + { + var headers = new Dictionary + { + {"ServiceControl.Retry.Successful", string.Empty}, + {"ServiceControl.Retry.UniqueMessageId", MessageId} + }; + var messageContext = new MessageContext( + MessageId, + headers, + new byte[0], + new TransportTransaction(), + new CancellationTokenSource(), + new ContextBag()); + + var messageContexts = new List + { + messageContext, + messageContext + }; + + var commands = Processor.Process(messageContexts); + + Assert.DoesNotThrowAsync(() => Store.AsyncDatabaseCommands.BatchAsync(commands)); + } + + [Test] + public async Task Should_handle_multiple_legacy_audit_instance_retry_confirmations() + { + var retryConfirmation = new MarkMessageFailureResolvedByRetry + { + FailedMessageId = MessageId + }; + + await Handler.Handle(retryConfirmation, new TestableMessageHandlerContext()); + + Assert.DoesNotThrowAsync(() => Handler.Handle(retryConfirmation, new TestableInvokeHandlerContext())); + } + + const string MessageId = "83C73A86-A45E-4FDF-8C95-E292526166F5"; + + } +} \ No newline at end of file From d1f7c68827fde8f93f5b8c41b8f5fd9581e4226f Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 13 Jul 2021 11:44:17 +0200 Subject: [PATCH 30/42] Moar ack tests --- .../RetryConfirmationProcessorTests.cs | 73 ++++++++++++++----- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs b/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs index ec14fc1b50..9315b62f58 100644 --- a/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs +++ b/src/ServiceControl.UnitTests/Recoverability/RetryConfirmationProcessorTests.cs @@ -14,7 +14,6 @@ using Raven.Client; using Raven.Tests.Helpers; using MessageFailures; - using Raven.Abstractions.Commands; using ServiceControl.Operations; using ServiceControl.Recoverability; @@ -54,41 +53,79 @@ public async Task Setup() [Test] public void Should_handle_multiple_retry_confirmations_in_the_error_ingestion() { - var headers = new Dictionary + var messageContexts = new List { - {"ServiceControl.Retry.Successful", string.Empty}, - {"ServiceControl.Retry.UniqueMessageId", MessageId} + CreateRetryAcknowledgementMessage(), + CreateRetryAcknowledgementMessage() }; - var messageContext = new MessageContext( - MessageId, - headers, - new byte[0], - new TransportTransaction(), - new CancellationTokenSource(), - new ContextBag()); + var commands = Processor.Process(messageContexts); + + Assert.DoesNotThrowAsync(() => Store.AsyncDatabaseCommands.BatchAsync(commands)); + } + + [Test] + public async Task Should_handle_multiple_legacy_audit_instance_retry_confirmations() + { + await Handler.Handle(CreateLegacyRetryConfirmationCommand(), new TestableMessageHandlerContext()); + + Assert.DoesNotThrowAsync( + () => Handler.Handle(CreateLegacyRetryConfirmationCommand(), new TestableInvokeHandlerContext())); + } + + [Test] + public async Task Should_handle_retry_confirmation_followed_by_legacy_command() + { var messageContexts = new List { - messageContext, - messageContext + CreateRetryAcknowledgementMessage() }; var commands = Processor.Process(messageContexts); + await Store.AsyncDatabaseCommands.BatchAsync(commands); - Assert.DoesNotThrowAsync(() => Store.AsyncDatabaseCommands.BatchAsync(commands)); + Assert.DoesNotThrowAsync( + () => Handler.Handle(CreateLegacyRetryConfirmationCommand(), new TestableInvokeHandlerContext())); } [Test] - public async Task Should_handle_multiple_legacy_audit_instance_retry_confirmations() + public async Task Should_handle_legacy_retry_confirmation_command_followed_by_new_acknowledgement() + { + await Handler.Handle(CreateLegacyRetryConfirmationCommand(), new TestableMessageHandlerContext()); + + var messageContexts = new List + { + CreateRetryAcknowledgementMessage() + }; + + var commands = Processor.Process(messageContexts); + Assert.DoesNotThrowAsync(() => Store.AsyncDatabaseCommands.BatchAsync(commands)); + } + + static MarkMessageFailureResolvedByRetry CreateLegacyRetryConfirmationCommand() { var retryConfirmation = new MarkMessageFailureResolvedByRetry { FailedMessageId = MessageId }; + return retryConfirmation; + } - await Handler.Handle(retryConfirmation, new TestableMessageHandlerContext()); - - Assert.DoesNotThrowAsync(() => Handler.Handle(retryConfirmation, new TestableInvokeHandlerContext())); + static MessageContext CreateRetryAcknowledgementMessage() + { + var headers = new Dictionary + { + {"ServiceControl.Retry.Successful", string.Empty}, + {"ServiceControl.Retry.UniqueMessageId", MessageId} + }; + var messageContext = new MessageContext( + MessageId, + headers, + new byte[0], + new TransportTransaction(), + new CancellationTokenSource(), + new ContextBag()); + return messageContext; } const string MessageId = "83C73A86-A45E-4FDF-8C95-E292526166F5"; From 68d808c3fdd9b831843acca1795e0cb9e071d793 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 13 Jul 2021 12:25:02 +0200 Subject: [PATCH 31/42] Include temp debug info --- .../SagaAudit/When_a_message_that_is_handled_by_a_saga.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs index 2de3da0410..b292e3ef24 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using AcceptanceTesting; using Audit.Auditing.MessagesView; + using Newtonsoft.Json; using NServiceBus; using NServiceBus.AcceptanceTesting; using NServiceBus.Pipeline; @@ -36,6 +37,7 @@ public async Task Message_should_be_enriched_with_saga_state_changes() messages = result; if (result) { + c.Messages = JsonConvert.SerializeObject(messages); return messages.Count == 5; } } @@ -198,6 +200,7 @@ public class MyContext : ScenarioContext public bool Saga2Complete { get; set; } public Guid Saga1Id { get; set; } public Guid Saga2Id { get; set; } + public string Messages { get; set; } } } } \ No newline at end of file From f2d630208f96d4f5f02fb570ebc59db96db6c333 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 13 Jul 2021 13:03:14 +0200 Subject: [PATCH 32/42] Make the session filter more specific --- .../TestSupport/ServiceControlComponentRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 927d8153ab..86be9eb638 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -93,7 +93,7 @@ async Task InitializeServiceControl(ScenarioContext context) //Do not filter out CC, SA and HB messages as they can't be stamped if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) - && messageTypes.StartsWith("ServiceControl.")) + && (messageTypes.StartsWith("ServiceControl.Contracts") || messageTypes.StartsWith("ServiceControl.EndpointPlugin"))) { return @continue(); } From b5719bb751ca9e5f2255a47d6fb48fd26f09e1a6 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Tue, 13 Jul 2021 13:46:51 +0200 Subject: [PATCH 33/42] Bing back handling of OnMessage callback --- .../Auditing/AuditIngestionComponent.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs b/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs index d74a8940a8..01e72902af 100644 --- a/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs +++ b/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs @@ -57,16 +57,21 @@ AuditIngestionCustomCheck.State ingestionState ingestor = new AuditIngestor(auditPersister, settings); var ingestion = new AuditIngestion( - async (messageContext, dispatcher) => - { - var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - messageContext.SetTaskCompletionSource(taskCompletionSource); + (messageContext, dispatcher) => + { + async Task Process() + { + var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + messageContext.SetTaskCompletionSource(taskCompletionSource); + + receivedMeter.Mark(); - receivedMeter.Mark(); + await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); + await taskCompletionSource.Task.ConfigureAwait(false); + } - await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); - await taskCompletionSource.Task.ConfigureAwait(false); - }, + return settings.OnMessage(messageContext.MessageId, messageContext.Headers, messageContext.Body, Process); + }, dispatcher => ingestor.Initialize(dispatcher), settings.AuditQueue, rawEndpointFactory, errorHandlingPolicy, OnCriticalError); From 258e5ac6fc1f3898a386a89200f04ba349e46bb3 Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Tue, 13 Jul 2021 14:18:21 +0200 Subject: [PATCH 34/42] fixing merge changes --- .../SagaRelationshipsEnricherTests.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.Audit.UnitTests/SagaAudit/SagaRelationshipsEnricherTests.cs b/src/ServiceControl.Audit.UnitTests/SagaAudit/SagaRelationshipsEnricherTests.cs index 31f65c0537..ca8f0cc1ff 100644 --- a/src/ServiceControl.Audit.UnitTests/SagaAudit/SagaRelationshipsEnricherTests.cs +++ b/src/ServiceControl.Audit.UnitTests/SagaAudit/SagaRelationshipsEnricherTests.cs @@ -4,6 +4,7 @@ using Audit.Auditing; using Audit.SagaAudit; using NServiceBus; + using NServiceBus.Transport; using NUnit.Framework; using ServiceControl.SagaAudit; @@ -23,8 +24,9 @@ public void New_overrides_Updated_state() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; @@ -45,8 +47,9 @@ public void Updated_does_not_override_new() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; @@ -67,8 +70,9 @@ public void Updated_does_not_override_completed() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; @@ -89,8 +93,9 @@ public void Completed_overrides_new() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; @@ -111,8 +116,9 @@ public void New_does_not_override_completed() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; @@ -133,8 +139,9 @@ public void It_can_parse_malformed_headers_of_three_sagas() var metadata = new Dictionary(); var commandsToEmit = new List(); + var transportOperations = new List(); - enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, metadata)); + enricher.Enrich(new AuditEnricherContext(headers, commandsToEmit, transportOperations, metadata)); var sagaData = (List)metadata["InvokedSagas"]; From 93a5b071f135a8a3e76df1de8f1c3142de10ae30 Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Tue, 13 Jul 2021 14:41:56 +0200 Subject: [PATCH 35/42] att message filtering --- .../ServiceControlComponentRunner.cs | 2 +- .../Operations/ErrorIngestionComponent.cs | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 6924ab6430..c25f4647cc 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -102,7 +102,7 @@ async Task InitializeServiceControl(ScenarioContext context) //Do not filter out CC, SA and HB messages as they can't be stamped if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) - && messageTypes.StartsWith("ServiceControl.")) + && (messageTypes.StartsWith("ServiceControl.Contracts") || messageTypes.StartsWith("ServiceControl.EndpointPlugin"))) { return @continue(); } diff --git a/src/ServiceControl/Operations/ErrorIngestionComponent.cs b/src/ServiceControl/Operations/ErrorIngestionComponent.cs index b6dc802c8c..cb1da6042a 100644 --- a/src/ServiceControl/Operations/ErrorIngestionComponent.cs +++ b/src/ServiceControl/Operations/ErrorIngestionComponent.cs @@ -54,17 +54,25 @@ ErrorIngestionCustomCheck.State ingestionState ingestor = new ErrorIngestor(errorProcessor, retryConfirmationProcessor, documentStore, bulkInsertDurationMeter, settings.ForwardErrorMessages, settings.ErrorLogQueue); - var ingestion = new ErrorIngestion(async messageContext => - { - var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - messageContext.SetTaskCompletionSource(taskCompletionSource); + async Task Process(MessageContext messageContext) + { + var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + messageContext.SetTaskCompletionSource(taskCompletionSource); - receivedMeter.Mark(); + receivedMeter.Mark(); + + await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); + await taskCompletionSource.Task.ConfigureAwait(false); + } - await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); - await taskCompletionSource.Task.ConfigureAwait(false); - }, - dispatcher => ingestor.Initialize(dispatcher), settings.ErrorQueue, rawEndpointFactory, documentStore, loggingSettings, OnCriticalError); + var ingestion = new ErrorIngestion( + messageContext => settings.OnMessage(messageContext.MessageId, messageContext.Headers, messageContext.Body, () => Process(messageContext)), + dispatcher => ingestor.Initialize(dispatcher), + settings.ErrorQueue, + rawEndpointFactory, + documentStore, + loggingSettings, + OnCriticalError); failedImporter = new ImportFailedErrors(documentStore, ingestor, rawEndpointFactory); From bf495c764fef905ff8fe1d9e5ebd7cdaed4e01d0 Mon Sep 17 00:00:00 2001 From: Tomasz Masternak Date: Wed, 14 Jul 2021 11:47:41 +0200 Subject: [PATCH 36/42] Update src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs Co-authored-by: Tim Bussmann --- .../SagaAudit/When_a_message_that_is_handled_by_a_saga.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs index b292e3ef24..bf76a865c4 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs @@ -37,7 +37,6 @@ public async Task Message_should_be_enriched_with_saga_state_changes() messages = result; if (result) { - c.Messages = JsonConvert.SerializeObject(messages); return messages.Count == 5; } } @@ -203,4 +202,4 @@ public class MyContext : ScenarioContext public string Messages { get; set; } } } -} \ No newline at end of file +} From b49688749c61762d8ef66645f9a5fd84e65780bb Mon Sep 17 00:00:00 2001 From: Tomasz Masternak Date: Wed, 14 Jul 2021 11:50:16 +0200 Subject: [PATCH 37/42] Apply suggestions from code review Co-authored-by: Tim Bussmann --- .../When_a_successful_retry_at_old_endpoint_is_detected.cs | 5 +++-- .../Recoverability/When_a_successful_retry_is_detected.cs | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs index 17e742819b..ae94220a8b 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_at_old_endpoint_is_detected.cs @@ -15,7 +15,7 @@ class When_a_successful_retry_at_old_endpoint_is_detected : AcceptanceTest { [Test] - public async Task Should_sent_acknowledgement() + public async Task Should_send_acknowledgement() { var failedMessageId = Guid.NewGuid().ToString(); var context = await Define() @@ -77,6 +77,7 @@ public Receiver() { EndpointSetup(cfg => { + // disable retry notifications feature to emulate an NServiceBus endpoint older than version 7.5 cfg.DisableFeature(); }); } @@ -94,4 +95,4 @@ public class MyMessage : ICommand { } } -} \ No newline at end of file +} diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs index 335693ce5a..2a66bf9f1d 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs @@ -14,7 +14,7 @@ class When_a_successful_retry_is_detected : AcceptanceTest { [Test] - public async Task Should_sent_acknowledgement() + public async Task Should_send_acknowledgement() { var failedMessageId = Guid.NewGuid().ToString(); var context = await Define() @@ -89,4 +89,4 @@ public class MyMessage : ICommand { } } -} \ No newline at end of file +} From ca5a18403680da0ad82a56cbd229f7bfc08daa61 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Wed, 14 Jul 2021 12:04:04 +0200 Subject: [PATCH 38/42] Replace acceptance test with a set of unit test for the detect successful retry behavior --- .../When_a_successful_retry_is_detected.cs | 92 ------------------- .../DetectSuccessfulRetriesEnricherTests.cs | 81 ++++++++++++++++ .../DetectSuccessfulRetriesEnricher.cs | 89 ++++++++++++++++++ .../Recoverability/FailedMessagesFeature.cs | 87 +----------------- 4 files changed, 171 insertions(+), 178 deletions(-) delete mode 100644 src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs create mode 100644 src/ServiceControl.Audit.UnitTests/Receoverability/DetectSuccessfulRetriesEnricherTests.cs create mode 100644 src/ServiceControl.Audit/Recoverability/DetectSuccessfulRetriesEnricher.cs diff --git a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs b/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs deleted file mode 100644 index 2a66bf9f1d..0000000000 --- a/src/ServiceControl.Audit.AcceptanceTests/Recoverability/When_a_successful_retry_is_detected.cs +++ /dev/null @@ -1,92 +0,0 @@ -namespace ServiceControl.Audit.AcceptanceTests.Recoverability -{ - using System; - using System.Threading.Tasks; - using AcceptanceTesting; - using NServiceBus; - using NServiceBus.AcceptanceTesting; - using NServiceBus.Pipeline; - using NUnit.Framework; - using TestSupport.EndpointTemplates; - using Conventions = NServiceBus.AcceptanceTesting.Customization.Conventions; - - [RunOnAllTransports] - class When_a_successful_retry_is_detected : AcceptanceTest - { - [Test] - public async Task Should_send_acknowledgement() - { - var failedMessageId = Guid.NewGuid().ToString(); - var context = await Define() - .WithEndpoint(b => b.When(s => - { - var options = new SendOptions(); - - options.SetHeader("ServiceControl.Retry.UniqueMessageId", failedMessageId); - options.SetHeader("ServiceControl.Retry.AcknowledgementQueue", Conventions.EndpointNamingConvention(typeof(AcknowledgementSpy))); - options.RouteToThisEndpoint(); - return s.Send(new MyMessage(), options); - })) - .WithEndpoint() - .Done(c => c.AcknowledgementSent) - .Run(); - - Assert.IsTrue(context.AcknowledgementSent); - } - - public class Context : ScenarioContext - { - public bool AcknowledgementSent { get; set; } - } - - public class AcknowledgementSpy : EndpointConfigurationBuilder - { - public AcknowledgementSpy() - { - EndpointSetup(cfg => - { - cfg.Pipeline.Register(b => new SpyBehavior(b.Build()), "Spy behavior"); - }); - } - - public class SpyBehavior : Behavior - { - Context scenarioContext; - - public SpyBehavior(Context scenarioContext) - { - this.scenarioContext = scenarioContext; - } - - public override Task Invoke(ITransportReceiveContext context, Func next) - { - if (context.Message.Headers.ContainsKey("ServiceControl.Retry.Successful")) - { - scenarioContext.AcknowledgementSent = true; - } - return Task.CompletedTask; - } - } - } - - public class Receiver : EndpointConfigurationBuilder - { - public Receiver() - { - EndpointSetup(); - } - - public class MyMessageHandler : IHandleMessages - { - public Task Handle(MyMessage message, IMessageHandlerContext context) - { - return Task.FromResult(0); - } - } - } - - public class MyMessage : ICommand - { - } - } -} diff --git a/src/ServiceControl.Audit.UnitTests/Receoverability/DetectSuccessfulRetriesEnricherTests.cs b/src/ServiceControl.Audit.UnitTests/Receoverability/DetectSuccessfulRetriesEnricherTests.cs new file mode 100644 index 0000000000..ef62085671 --- /dev/null +++ b/src/ServiceControl.Audit.UnitTests/Receoverability/DetectSuccessfulRetriesEnricherTests.cs @@ -0,0 +1,81 @@ +namespace ServiceControl.Audit.UnitTests.Receoverability +{ + using System.Collections.Generic; + using System.Linq; + using Audit.Auditing; + using Contracts.MessageFailures; + using NServiceBus; + using NServiceBus.Transport; + using NUnit.Framework; + using Recoverability; + + [TestFixture] + public class DetectSuccessfulRetriesEnricherTests + { + [Test] + public void It_does_not_sent_acknowledgement_if_audit_comes_from_new_endpoint_version() + { + var enricher = new DetectSuccessfulRetriesEnricher(); + + var headers = new Dictionary + { + ["ServiceControl.Retry.UniqueMessageId"] = "MyId", + ["ServiceControl.Retry.AcknowledgementQueue"] = "ErrorQueue", + ["ServiceControl.Retry.AcknowledgementSent"] = "true" + }; + + var outgoingCommands = new List(); + var transportOperations = new List(); + var metadata = new Dictionary(); + + enricher.Enrich(new AuditEnricherContext(headers, outgoingCommands, transportOperations, metadata)); + + CollectionAssert.IsEmpty(outgoingCommands); + CollectionAssert.IsEmpty(transportOperations); + } + + [Test] + public void It_sends_legacy_command_if_retry_comes_from_old_ServiceControl() + { + var enricher = new DetectSuccessfulRetriesEnricher(); + + var headers = new Dictionary + { + ["ServiceControl.Retry.UniqueMessageId"] = "MyId" + }; + + var outgoingCommands = new List(); + var transportOperations = new List(); + var metadata = new Dictionary(); + + enricher.Enrich(new AuditEnricherContext(headers, outgoingCommands, transportOperations, metadata)); + + CollectionAssert.IsNotEmpty(outgoingCommands); + CollectionAssert.AllItemsAreInstancesOfType(outgoingCommands, typeof(MarkMessageFailureResolvedByRetry)); + CollectionAssert.IsEmpty(transportOperations); + } + + [Test] + public void It_sends_acknowledgement_if_audit_comes_from_old_endpoint_version() + { + var enricher = new DetectSuccessfulRetriesEnricher(); + + var headers = new Dictionary + { + ["ServiceControl.Retry.UniqueMessageId"] = "MyId", + ["ServiceControl.Retry.AcknowledgementQueue"] = "ErrorQueue" + }; + + var outgoingCommands = new List(); + var transportOperations = new List(); + var metadata = new Dictionary(); + + enricher.Enrich(new AuditEnricherContext(headers, outgoingCommands, transportOperations, metadata)); + + CollectionAssert.IsEmpty(outgoingCommands); + + var ack = transportOperations.Single(); + Assert.IsTrue(ack.Message.Headers.ContainsKey("ServiceControl.Retry.Successful")); + } + } +} diff --git a/src/ServiceControl.Audit/Recoverability/DetectSuccessfulRetriesEnricher.cs b/src/ServiceControl.Audit/Recoverability/DetectSuccessfulRetriesEnricher.cs new file mode 100644 index 0000000000..2488e6affa --- /dev/null +++ b/src/ServiceControl.Audit/Recoverability/DetectSuccessfulRetriesEnricher.cs @@ -0,0 +1,89 @@ +namespace ServiceControl.Audit.Recoverability +{ + using System; + using System.Collections.Generic; + using System.Linq; + using Auditing; + using Contracts.MessageFailures; + using Infrastructure; + using NServiceBus; + using NServiceBus.Routing; + using NServiceBus.Transport; + + class DetectSuccessfulRetriesEnricher : IEnrichImportedAuditMessages + { + public void Enrich(AuditEnricherContext context) + { + var headers = context.Headers; + var isOldRetry = headers.TryGetValue("ServiceControl.RetryId", out _); + var isNewRetry = headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var newRetryMessageId); + var isAckHandled = headers.ContainsKey("ServiceControl.Retry.AcknowledgementSent"); + var hasAckQueue = headers.TryGetValue("ServiceControl.Retry.AcknowledgementQueue", out var ackQueue); + + var hasBeenRetried = isOldRetry || isNewRetry; + + context.Metadata.Add("IsRetried", hasBeenRetried); + + if (!hasBeenRetried || isAckHandled) + { + //The message has not been sent for retry from ServiceControl or the endpoint indicated that is already has sent a retry acknowledgement to the + //ServiceControl main instance. Nothing to do. + return; + } + + if (hasAckQueue && isNewRetry) + { + //The message has been sent for retry from ServiceControl 4.20 or higher (has the ACK queue header) but the endpoint did not recognize the header + //and did not send the acknowledgment. We send it here to the acknowledgment queue. + var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary + { + ["ServiceControl.Retry.Successful"] = DateTimeExtensions.ToWireFormattedString(DateTime.UtcNow), + ["ServiceControl.Retry.UniqueMessageId"] = newRetryMessageId + }, new byte[0]); + var ackOperation = new TransportOperation(ackMessage, new UnicastAddressTag(ackQueue)); + context.AddForSend(ackOperation); + } + else + { + //The message has been sent for retry from ServiceControl older than 4.20. Regardless which version the endpoint was, we need to send a legacy confirmation + //message because the main instance of ServiceControl may still be on version lower than 4.19. + context.AddForSend(new MarkMessageFailureResolvedByRetry + { + FailedMessageId = isOldRetry ? headers.UniqueId() : newRetryMessageId, + AlternativeFailedMessageIds = GetAlternativeUniqueMessageId(headers).ToArray() + }); + } + } + + IEnumerable GetAlternativeUniqueMessageId(IReadOnlyDictionary headers) + { + var messageId = headers.MessageId(); + if (headers.TryGetValue(Headers.ProcessingEndpoint, out var processingEndpoint)) + { + yield return DeterministicGuid.MakeId(messageId, processingEndpoint).ToString(); + } + + if (headers.TryGetValue("NServiceBus.FailedQ", out var failedQ)) + { + yield return DeterministicGuid.MakeId(messageId, ExtractQueueNameForLegacyReasons(failedQ)).ToString(); + } + + if (headers.TryGetValue(Headers.ReplyToAddress, out var replyToAddress)) + { + yield return DeterministicGuid.MakeId(messageId, ExtractQueueNameForLegacyReasons(replyToAddress)).ToString(); + } + } + + static string ExtractQueueNameForLegacyReasons(string address) + { + var atIndex = address?.IndexOf("@", StringComparison.InvariantCulture); + + if (atIndex.HasValue && atIndex.Value > -1) + { + return address.Substring(0, atIndex.Value); + } + + return address; + } + } +} \ No newline at end of file diff --git a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs index afbc538768..c3237747c8 100644 --- a/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs +++ b/src/ServiceControl.Audit/Recoverability/FailedMessagesFeature.cs @@ -1,15 +1,7 @@ namespace ServiceControl.Audit.Recoverability { - using System; - using System.Collections.Generic; - using System.Linq; - using Auditing; - using Contracts.MessageFailures; - using Infrastructure; using NServiceBus; using NServiceBus.Features; - using NServiceBus.Routing; - using NServiceBus.Transport; class FailedMessagesFeature : Feature { @@ -20,84 +12,7 @@ public FailedMessagesFeature() protected override void Setup(FeatureConfigurationContext context) { - context.Container.ConfigureComponent(DependencyLifecycle.SingleInstance); - } - - class DetectSuccessfullRetriesEnricher : IEnrichImportedAuditMessages - { - public void Enrich(AuditEnricherContext context) - { - var headers = context.Headers; - var isOldRetry = headers.TryGetValue("ServiceControl.RetryId", out _); - var isNewRetry = headers.TryGetValue("ServiceControl.Retry.UniqueMessageId", out var newRetryMessageId); - var isAckHandled = headers.ContainsKey("ServiceControl.Retry.AcknowledgementSent"); - var hasAckQueue = headers.TryGetValue("ServiceControl.Retry.AcknowledgementQueue", out var ackQueue); - - var hasBeenRetried = isOldRetry || isNewRetry; - - context.Metadata.Add("IsRetried", hasBeenRetried); - - if (!hasBeenRetried || isAckHandled) - { - //The message has not been sent for retry from ServiceControl or the endpoint indicated that is already has sent a retry acknowledgement to the - //ServiceControl main instance. Nothing to do. - return; - } - - if (hasAckQueue && isNewRetry) - { - //The message has been sent for retry from ServiceControl 4.20 or higher (has the ACK queue header) but the endpoint did not recognize the header - //and did not send the acknowledgment. We send it here to the acknowledgment queue. - var ackMessage = new OutgoingMessage(Guid.NewGuid().ToString(), new Dictionary - { - ["ServiceControl.Retry.Successful"] = DateTimeExtensions.ToWireFormattedString(DateTime.UtcNow), - ["ServiceControl.Retry.UniqueMessageId"] = newRetryMessageId - }, new byte[0]); - var ackOperation = new TransportOperation(ackMessage, new UnicastAddressTag(ackQueue)); - context.AddForSend(ackOperation); - } - else - { - //The message has been sent for retry from ServiceControl older than 4.20. Regardless which version the endpoint was, we need to send a legacy confirmation - //message because the main instance of ServiceControl may still be on version lower than 4.19. - context.AddForSend(new MarkMessageFailureResolvedByRetry - { - FailedMessageId = isOldRetry ? headers.UniqueId() : newRetryMessageId, - AlternativeFailedMessageIds = GetAlternativeUniqueMessageId(headers).ToArray() - }); - } - } - - IEnumerable GetAlternativeUniqueMessageId(IReadOnlyDictionary headers) - { - var messageId = headers.MessageId(); - if (headers.TryGetValue(Headers.ProcessingEndpoint, out var processingEndpoint)) - { - yield return DeterministicGuid.MakeId(messageId, processingEndpoint).ToString(); - } - - if (headers.TryGetValue("NServiceBus.FailedQ", out var failedQ)) - { - yield return DeterministicGuid.MakeId(messageId, ExtractQueueNameForLegacyReasons(failedQ)).ToString(); - } - - if (headers.TryGetValue(Headers.ReplyToAddress, out var replyToAddress)) - { - yield return DeterministicGuid.MakeId(messageId, ExtractQueueNameForLegacyReasons(replyToAddress)).ToString(); - } - } - - static string ExtractQueueNameForLegacyReasons(string address) - { - var atIndex = address?.IndexOf("@", StringComparison.InvariantCulture); - - if (atIndex.HasValue && atIndex.Value > -1) - { - return address.Substring(0, atIndex.Value); - } - - return address; - } + context.Container.ConfigureComponent(DependencyLifecycle.SingleInstance); } } } From 30c79dd32095204081ce16a284945ce75a681975 Mon Sep 17 00:00:00 2001 From: Tomek Masternak Date: Wed, 14 Jul 2021 12:31:03 +0200 Subject: [PATCH 39/42] refactoring message filtering logic for acceptance tests --- .../ServiceControlComponentRunner.cs | 12 +++++---- ...hen_a_message_that_is_handled_by_a_saga.cs | 1 - .../ServiceControlComponentRunner.cs | 13 ++++++---- .../Auditing/AuditIngestionComponent.cs | 18 ++++++------- .../Infrastructure/Settings/Settings.cs | 5 +++- .../ServiceControlComponentRunner.cs | 25 +++++++++++-------- .../Infrastructure/Settings/Settings.cs | 8 +++--- .../Operations/ErrorIngestionComponent.cs | 23 +++++++++-------- 8 files changed, 61 insertions(+), 44 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index c25f4647cc..6e3733a97d 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -94,8 +94,10 @@ async Task InitializeServiceControl(ScenarioContext context) RunInMemory = true, DisableHealthChecks = true, ExposeApi = true, - OnMessage = (id, headers, body, @continue) => + MessageFilter = messageContext => { + var headers = messageContext.Headers; + var id = messageContext.MessageId; var log = LogManager.GetLogger(); headers.TryGetValue(Headers.MessageId, out var originalMessageId); log.Debug($"OnMessage for message '{id}'({originalMessageId ?? string.Empty})."); @@ -104,24 +106,24 @@ async Task InitializeServiceControl(ScenarioContext context) if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) && (messageTypes.StartsWith("ServiceControl.Contracts") || messageTypes.StartsWith("ServiceControl.EndpointPlugin"))) { - return @continue(); + return false; } //Do not filter out subscribe messages as they can't be stamped if (headers.TryGetValue(Headers.MessageIntent, out var intent) && intent == MessageIntentEnum.Subscribe.ToString()) { - return @continue(); + return false; } var currentSession = context.TestRunId.ToString(); if (!headers.TryGetValue("SC.SessionID", out var session) || session != currentSession) { log.Debug($"Discarding message '{id}'({originalMessageId ?? string.Empty}) because it's session id is '{session}' instead of '{currentSession}'."); - return Task.FromResult(0); + return true; } - return @continue(); + return false; } }; diff --git a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs index bf76a865c4..ce28d33917 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/SagaAudit/When_a_message_that_is_handled_by_a_saga.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using AcceptanceTesting; using Audit.Auditing.MessagesView; - using Newtonsoft.Json; using NServiceBus; using NServiceBus.AcceptanceTesting; using NServiceBus.Pipeline; diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 86be9eb638..11e35081ed 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -85,8 +85,11 @@ async Task InitializeServiceControl(ScenarioContext context) RunInMemory = true, ExposeApi = false, ServiceControlQueueAddress = "SHOULDNOTBEUSED", - OnMessage = (id, headers, body, @continue) => + MessageFilter = messageContext => { + var id = messageContext.MessageId; + var headers = messageContext.Headers; + var log = LogManager.GetLogger(); headers.TryGetValue(Headers.MessageId, out var originalMessageId); log.Debug($"OnMessage for message '{id}'({originalMessageId ?? string.Empty})."); @@ -95,24 +98,24 @@ async Task InitializeServiceControl(ScenarioContext context) if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) && (messageTypes.StartsWith("ServiceControl.Contracts") || messageTypes.StartsWith("ServiceControl.EndpointPlugin"))) { - return @continue(); + return false; } //Do not filter out subscribe messages as they can't be stamped if (headers.TryGetValue(Headers.MessageIntent, out var intent) && intent == MessageIntentEnum.Subscribe.ToString()) { - return @continue(); + return false; } var currentSession = context.TestRunId.ToString(); if (!headers.TryGetValue("SC.SessionID", out var session) || session != currentSession) { log.Debug($"Discarding message '{id}'({originalMessageId ?? string.Empty}) because it's session id is '{session}' instead of '{currentSession}'."); - return Task.FromResult(0); + return true; } - return @continue(); + return false; } }; diff --git a/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs b/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs index 01e72902af..9def0f4e7b 100644 --- a/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs +++ b/src/ServiceControl.Audit/Auditing/AuditIngestionComponent.cs @@ -57,20 +57,20 @@ AuditIngestionCustomCheck.State ingestionState ingestor = new AuditIngestor(auditPersister, settings); var ingestion = new AuditIngestion( - (messageContext, dispatcher) => + async (messageContext, dispatcher) => { - async Task Process() + if (settings.MessageFilter != null && settings.MessageFilter(messageContext)) { - var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - messageContext.SetTaskCompletionSource(taskCompletionSource); + return; + } - receivedMeter.Mark(); + var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + messageContext.SetTaskCompletionSource(taskCompletionSource); - await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); - await taskCompletionSource.Task.ConfigureAwait(false); - } + receivedMeter.Mark(); - return settings.OnMessage(messageContext.MessageId, messageContext.Headers, messageContext.Body, Process); + await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); + await taskCompletionSource.Task.ConfigureAwait(false); }, dispatcher => ingestor.Initialize(dispatcher), settings.AuditQueue, rawEndpointFactory, errorHandlingPolicy, OnCriticalError); diff --git a/src/ServiceControl.Audit/Infrastructure/Settings/Settings.cs b/src/ServiceControl.Audit/Infrastructure/Settings/Settings.cs index 02dbab2821..f0b3491d3a 100644 --- a/src/ServiceControl.Audit/Infrastructure/Settings/Settings.cs +++ b/src/ServiceControl.Audit/Infrastructure/Settings/Settings.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using NLog.Common; using NServiceBus.Logging; + using NServiceBus.Transport; using Transports; public class Settings @@ -46,8 +47,10 @@ public Settings(string serviceName = null) EnableFullTextSearchOnBodies = SettingsReader.Read("EnableFullTextSearchOnBodies", true); } - public Func, byte[], Func, Task> OnMessage { get; set; } = (messageId, headers, body, next) => next(); + //HINT: acceptance tests only + public Func MessageFilter { get; set; } + //HINT: acceptance tests only public bool RunInMemory { get; set; } public bool ValidateConfiguration => SettingsReader.Read("ValidateConfig", true); diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index c0aec8883b..7b68b63c60 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -108,8 +108,10 @@ async Task InitializeServiceControl(ScenarioContext context, int instancePort, i ApiUri = $"http://localhost:{auditInstanceApiPort}/api" // evil assumption for now } }, - OnMessage = (id, headers, body, @continue) => + MessageFilter = messageContext => { + var headers = messageContext.Headers; + var id = messageContext.MessageId; var log = LogManager.GetLogger(); headers.TryGetValue(Headers.MessageId, out var originalMessageId); log.Debug($"OnMessage for message '{id}'({originalMessageId ?? string.Empty})."); @@ -118,24 +120,24 @@ async Task InitializeServiceControl(ScenarioContext context, int instancePort, i if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) && messageTypes.StartsWith("ServiceControl.")) { - return @continue(); + return false; } //Do not filter out subscribe messages as they can't be stamped if (headers.TryGetValue(Headers.MessageIntent, out var intent) && intent == MessageIntentEnum.Subscribe.ToString()) { - return @continue(); + return false; } var currentSession = context.TestRunId.ToString(); if (!headers.TryGetValue("SC.SessionID", out var session) || session != currentSession) { log.Debug($"Discarding message '{id}'({originalMessageId ?? string.Empty}) because it's session id is '{session}' instead of '{currentSession}'."); - return Task.FromResult(0); + return true; } - return @continue(); + return false; } }; @@ -235,8 +237,11 @@ async Task InitializeServiceControlAudit(ScenarioContext context, int instancePo RunInMemory = true, ExposeApi = false, ServiceControlQueueAddress = Settings.DEFAULT_SERVICE_NAME, - OnMessage = (id, headers, body, @continue) => + MessageFilter = messageContext => { + var id = messageContext.MessageId; + var headers = messageContext.Headers; + var log = LogManager.GetLogger(); headers.TryGetValue(Headers.MessageId, out var originalMessageId); log.Debug($"OnMessage for message '{id}'({originalMessageId ?? string.Empty})."); @@ -245,24 +250,24 @@ async Task InitializeServiceControlAudit(ScenarioContext context, int instancePo if (headers.TryGetValue(Headers.EnclosedMessageTypes, out var messageTypes) && messageTypes.StartsWith("ServiceControl.")) { - return @continue(); + return false; } //Do not filter out subscribe messages as they can't be stamped if (headers.TryGetValue(Headers.MessageIntent, out var intent) && intent == MessageIntentEnum.Subscribe.ToString()) { - return @continue(); + return false; } var currentSession = context.TestRunId.ToString(); if (!headers.TryGetValue("SC.SessionID", out var session) || session != currentSession) { log.Debug($"Discarding message '{id}'({originalMessageId ?? string.Empty}) because it's session id is '{session}' instead of '{currentSession}'."); - return Task.FromResult(0); + return true; } - return @continue(); + return false; } }; diff --git a/src/ServiceControl/Infrastructure/Settings/Settings.cs b/src/ServiceControl/Infrastructure/Settings/Settings.cs index abf35793a9..bf3220b0b0 100644 --- a/src/ServiceControl/Infrastructure/Settings/Settings.cs +++ b/src/ServiceControl/Infrastructure/Settings/Settings.cs @@ -1,15 +1,14 @@ namespace ServiceBus.Management.Infrastructure.Settings { using System; - using System.Collections.Generic; using System.Configuration; using System.IO; using System.Linq; using System.Reflection; - using System.Threading.Tasks; using Newtonsoft.Json; using NLog.Common; using NServiceBus.Logging; + using NServiceBus.Transport; using ServiceControl.Infrastructure.WebApi; using ServiceControl.Transports; @@ -59,10 +58,13 @@ public Settings(string serviceName = null) public bool AllowMessageEditing { get; set; } - public Func, byte[], Func, Task> OnMessage { get; set; } = (messageId, headers, body, next) => next(); + //HINT: acceptance tests only + public Func MessageFilter { get; set; } + //HINT: acceptance tests only public bool RunInMemory { get; set; } + //HINT: acceptance tests only public string EmailDropFolder { get; set; } public bool ValidateConfiguration => SettingsReader.Read("ValidateConfig", true); diff --git a/src/ServiceControl/Operations/ErrorIngestionComponent.cs b/src/ServiceControl/Operations/ErrorIngestionComponent.cs index cb1da6042a..1927c671ac 100644 --- a/src/ServiceControl/Operations/ErrorIngestionComponent.cs +++ b/src/ServiceControl/Operations/ErrorIngestionComponent.cs @@ -54,19 +54,22 @@ ErrorIngestionCustomCheck.State ingestionState ingestor = new ErrorIngestor(errorProcessor, retryConfirmationProcessor, documentStore, bulkInsertDurationMeter, settings.ForwardErrorMessages, settings.ErrorLogQueue); - async Task Process(MessageContext messageContext) - { - var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - messageContext.SetTaskCompletionSource(taskCompletionSource); + var ingestion = new ErrorIngestion( + async messageContext => + { + if (settings.MessageFilter != null && settings.MessageFilter(messageContext)) + { + return; + } - receivedMeter.Mark(); + var taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + messageContext.SetTaskCompletionSource(taskCompletionSource); - await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); - await taskCompletionSource.Task.ConfigureAwait(false); - } + receivedMeter.Mark(); - var ingestion = new ErrorIngestion( - messageContext => settings.OnMessage(messageContext.MessageId, messageContext.Headers, messageContext.Body, () => Process(messageContext)), + await channel.Writer.WriteAsync(messageContext).ConfigureAwait(false); + await taskCompletionSource.Task.ConfigureAwait(false); + }, dispatcher => ingestor.Initialize(dispatcher), settings.ErrorQueue, rawEndpointFactory, From 5c9a1ddc39c2d629491d5dfe29b93d07a5785821 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Wed, 14 Jul 2021 13:00:48 +0200 Subject: [PATCH 40/42] Make ack queue address take into account transport settings such as machine name --- .../When_a_retry_fails_to_be_sent.cs | 9 ++++---- .../Recoverability/Retry_State_Tests.cs | 13 +++++------ .../ReturnToSenderDequeuerTests.cs | 13 +++++------ .../Recoverability/RecoverabilityComponent.cs | 3 ++- .../Retrying/Infrastructure/ReturnToSender.cs | 9 +++----- .../Infrastructure/ReturnToSenderDequeuer.cs | 23 ++++++++++++++++--- 6 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs index a45109d3cc..768f817bbe 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_a_retry_fails_to_be_sent.cs @@ -13,7 +13,6 @@ using NUnit.Framework; using Operations.BodyStorage; using Raven.Client; - using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.MessageFailures; using ServiceControl.MessageFailures.Api; using ServiceControl.Recoverability; @@ -27,7 +26,7 @@ public async Task SubsequentBatchesShouldBeProcessed() { FailedMessage decomissionedFailure = null, successfullyRetried = null; - CustomConfiguration = config => config.RegisterComponents(components => components.ConfigureComponent(b => new FakeReturnToSender(b.Build(), b.Build(), b.Build(), b.Build()), DependencyLifecycle.SingleInstance)); + CustomConfiguration = config => config.RegisterComponents(components => components.ConfigureComponent(b => new FakeReturnToSender(b.Build(), b.Build(), b.Build()), DependencyLifecycle.SingleInstance)); await Define() .WithEndpoint(b => b.DoNotFailOnErrorMessages() @@ -149,19 +148,19 @@ public class MessageThatWillFail : ICommand public class FakeReturnToSender : ReturnToSender { - public FakeReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, Settings settings, MyContext myContext) : base(bodyStorage, documentStore, settings) + public FakeReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, MyContext myContext) : base(bodyStorage, documentStore) { this.myContext = myContext; } - public override Task HandleMessage(MessageContext message, IDispatchMessages sender) + public override Task HandleMessage(MessageContext message, IDispatchMessages sender, string errorQueueTransportAddress) { if (message.Headers[Headers.MessageId] == myContext.DecommissionedEndpointMessageId) { throw new Exception("This endpoint is unreachable"); } - return base.HandleMessage(message, sender); + return base.HandleMessage(message, sender, "error"); } MyContext myContext; diff --git a/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs b/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs index 427f1c456d..d954c3f415 100644 --- a/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs +++ b/src/ServiceControl.UnitTests/Recoverability/Retry_State_Tests.cs @@ -13,7 +13,6 @@ using NUnit.Framework; using Operations; using Raven.Client; - using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.Infrastructure.BackgroundTasks; using ServiceControl.Infrastructure.DomainEvents; using ServiceControl.Operations.BodyStorage.RavenAttachments; @@ -78,7 +77,7 @@ public async Task When_a_group_is_prepared_with_three_batches_and_SC_is_restarte var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"), retryManager); + var processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"), retryManager); documentStore.WaitForIndexing(); @@ -95,7 +94,7 @@ public async Task When_a_group_is_prepared_with_three_batches_and_SC_is_restarte await documentManager.RebuildRetryOperationState(session); - processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"), retryManager); + processor = new RetryProcessor(documentStore, domainEvents, new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"), retryManager); await processor.ProcessBatches(session, sender); await session.SaveChangesAsync(); @@ -120,7 +119,7 @@ public async Task When_a_group_is_forwarded_the_status_is_Completed() var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"); + var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"); var processor = new RetryProcessor(documentStore, domainEvents, returnToSender, retryManager); using (var session = documentStore.OpenAsyncSession()) @@ -161,7 +160,7 @@ public async Task When_there_is_one_poison_message_it_is_removed_from_batch_and_ var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore, new Settings()), documentStore, domainEvents, "TestEndpoint"); + var returnToSender = new TestReturnToSenderDequeuer(new ReturnToSender(bodyStorage, documentStore), documentStore, domainEvents, "TestEndpoint"); var processor = new RetryProcessor(documentStore, domainEvents, returnToSender, retryManager); bool c; @@ -204,7 +203,7 @@ public async Task When_a_group_has_one_batch_out_of_two_forwarded_the_status_is_ var bodyStorage = new RavenAttachmentsBodyStorage(documentStore); - var returnToSender = new ReturnToSender(bodyStorage, documentStore, new Settings()); + var returnToSender = new ReturnToSender(bodyStorage, documentStore); var sender = new TestSender(); @@ -319,7 +318,7 @@ public override Task MoveBatchToStaging(string batchDocumentId) class TestReturnToSenderDequeuer : ReturnToSenderDequeuer { public TestReturnToSenderDequeuer(ReturnToSender returnToSender, IDocumentStore store, IDomainEvents domainEvents, string endpointName) - : base(returnToSender, store, domainEvents, endpointName, null /* rawEndpointFactory */) + : base(returnToSender, store, domainEvents, endpointName, null /* rawEndpointFactory */, "error") { } diff --git a/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs b/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs index 63b5944ada..6b4103821a 100644 --- a/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs +++ b/src/ServiceControl.UnitTests/Recoverability/ReturnToSenderDequeuerTests.cs @@ -11,7 +11,6 @@ using NServiceBus.Extensibility; using NServiceBus.Transport; using NUnit.Framework; - using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.CompositeViews.Messages; using ServiceControl.Operations.BodyStorage; using ServiceControl.Recoverability; @@ -43,7 +42,7 @@ public async Task It_removes_staging_id_header() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender, "error") .ConfigureAwait(false); Assert.IsFalse(sender.Message.Headers.ContainsKey("ServiceControl.Retry.StagingId")); @@ -62,7 +61,7 @@ public async Task It_fetches_the_body_from_storage_if_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender, "error") .ConfigureAwait(false); Assert.AreEqual("MessageBodyId", Encoding.UTF8.GetString(sender.Message.Body)); @@ -111,7 +110,7 @@ await session.StoreAsync(new FailedMessage documentStore.WaitForIndexing(); - await new ReturnToSender(null, documentStore, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(null, documentStore).HandleMessage(message, sender, "error") .ConfigureAwait(false); Assert.AreEqual("MessageBodyId", Encoding.UTF8.GetString(sender.Message.Body)); @@ -131,7 +130,7 @@ public async Task It_uses_retry_to_if_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender, "error") .ConfigureAwait(false); Assert.AreEqual("Proxy", sender.Destination); @@ -150,7 +149,7 @@ public async Task It_sends_directly_to_target_if_retry_to_is_not_provided() }; var message = CreateMessage(Guid.NewGuid().ToString(), headers); - await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender, "error") .ConfigureAwait(false); Assert.AreEqual("TargetEndpoint", sender.Destination); @@ -172,7 +171,7 @@ public async Task It_restores_body_id_and_target_addres_after_failure() try { - await new ReturnToSender(new FakeBodyStorage(), null, new Settings()).HandleMessage(message, sender) + await new ReturnToSender(new FakeBodyStorage(), null).HandleMessage(message, sender, "error") .ConfigureAwait(false); } catch (Exception) diff --git a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs index aca4222c47..ae40b0ea9d 100644 --- a/src/ServiceControl/Recoverability/RecoverabilityComponent.cs +++ b/src/ServiceControl/Recoverability/RecoverabilityComponent.cs @@ -59,7 +59,8 @@ public override void Configure(Settings settings, IHostBuilder hostBuilder) b.GetRequiredService(), b.GetRequiredService(), stagingQueue, - b.GetRequiredService())); + b.GetRequiredService(), + settings.ErrorQueue)); collection.AddHostedService(); //Error importer diff --git a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs index ab28728b29..bd14dbf270 100644 --- a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs +++ b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSender.cs @@ -13,23 +13,21 @@ namespace ServiceControl.Recoverability using Operations.BodyStorage; using Raven.Client; using Raven.Json.Linq; - using ServiceBus.Management.Infrastructure.Settings; class ReturnToSender { - public ReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore, Settings settings) + public ReturnToSender(IBodyStorage bodyStorage, IDocumentStore documentStore) { this.documentStore = documentStore; - this.settings = settings; this.bodyStorage = bodyStorage; } - public virtual async Task HandleMessage(MessageContext message, IDispatchMessages sender) + public virtual async Task HandleMessage(MessageContext message, IDispatchMessages sender, string errorQueueTransportAddress) { var outgoingHeaders = new Dictionary(message.Headers); outgoingHeaders.Remove("ServiceControl.Retry.StagingId"); - outgoingHeaders["ServiceControl.Retry.AcknowledgementQueue"] = settings.ErrorQueue; + outgoingHeaders["ServiceControl.Retry.AcknowledgementQueue"] = errorQueueTransportAddress; byte[] body = null; var messageId = message.MessageId; @@ -150,6 +148,5 @@ async Task FetchFromBodyStore(string attemptMessageId, string messageId) readonly IBodyStorage bodyStorage; static readonly ILog Log = LogManager.GetLogger(typeof(ReturnToSender)); readonly IDocumentStore documentStore; - readonly Settings settings; } } diff --git a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSenderDequeuer.cs b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSenderDequeuer.cs index 45e96e78f3..73d3ccfb51 100644 --- a/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSenderDequeuer.cs +++ b/src/ServiceControl/Recoverability/Retrying/Infrastructure/ReturnToSenderDequeuer.cs @@ -5,17 +5,20 @@ namespace ServiceControl.Recoverability using System.Threading.Tasks; using Infrastructure.DomainEvents; using MessageFailures; + using NServiceBus; using NServiceBus.Logging; using NServiceBus.Raw; + using NServiceBus.Routing; using NServiceBus.Transport; using Raven.Client; class ReturnToSenderDequeuer { - public ReturnToSenderDequeuer(ReturnToSender returnToSender, IDocumentStore store, IDomainEvents domainEvents, string inputAddress, RawEndpointFactory rawEndpointFactory) + public ReturnToSenderDequeuer(ReturnToSender returnToSender, IDocumentStore store, IDomainEvents domainEvents, string inputAddress, RawEndpointFactory rawEndpointFactory, string errorQueue) { InputAddress = inputAddress; this.returnToSender = returnToSender; + this.errorQueue = errorQueue; createEndpointConfiguration = () => { @@ -45,7 +48,7 @@ async Task Handle(MessageContext message, IDispatchMessages sender) if (shouldProcess(message)) { - await returnToSender.HandleMessage(message, sender).ConfigureAwait(false); + await returnToSender.HandleMessage(message, sender, errorQueueTransportAddress).ConfigureAwait(false); IncrementCounterOrProlongTimer(); } else @@ -117,7 +120,11 @@ public virtual async Task Run(string forwardingBatchId, Predicate(); registration = cancellationToken.Register(() => _ = Task.Run(() => syncEvent.TrySetResult(true), CancellationToken.None)); - processor = await RawEndpoint.Start(config).ConfigureAwait(false); + var startable = await RawEndpoint.Create(config).ConfigureAwait(false); + + errorQueueTransportAddress = GetErrorQueueTransportAddress(startable); + + processor = await startable.Start().ConfigureAwait(false); Log.Info($"Forwarder for batch {forwardingBatchId} started receiving messages from {processor.TransportAddress}."); @@ -156,6 +163,13 @@ public virtual async Task Run(string forwardingBatchId, Predicate(); + var localInstance = transportInfra.BindToLocalEndpoint(new EndpointInstance(errorQueue)); + return transportInfra.ToTransportAddress(LogicalAddress.CreateRemoteAddress(localInstance)); + } + public Task Stop() { timer.Dispose(); @@ -188,6 +202,9 @@ async Task StopInternal() CaptureIfMessageSendingFails faultManager; Func createEndpointConfiguration; ReturnToSender returnToSender; + readonly string errorQueue; + string errorQueueTransportAddress; + static ILog Log = LogManager.GetLogger(typeof(ReturnToSenderDequeuer)); class CaptureIfMessageSendingFails : IErrorHandlingPolicy From bfadac447f4d8c44548b047cbb4392a2e82c6473 Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Wed, 14 Jul 2021 13:11:31 +0200 Subject: [PATCH 41/42] Better initial list size --- src/ServiceControl/Operations/RetryConfirmationProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs index 7a357bb272..dc659692ac 100644 --- a/src/ServiceControl/Operations/RetryConfirmationProcessor.cs +++ b/src/ServiceControl/Operations/RetryConfirmationProcessor.cs @@ -22,7 +22,7 @@ public RetryConfirmationProcessor(IDomainEvents domainEvents) public IReadOnlyCollection Process(List contexts) { - var allCommands = new List(contexts.Count); + var allCommands = new List(contexts.Count * 2); foreach (var context in contexts) { From 9498d04fe2c295118bf58d3443522824fc5e28ca Mon Sep 17 00:00:00 2001 From: Szymon Pobiega Date: Wed, 14 Jul 2021 14:19:38 +0200 Subject: [PATCH 42/42] Fix approvals --- ...APIApprovals.PlatformSampleSettings.approved.txt | 13 +------------ .../APIApprovals.PublicClr.approved.txt | 2 +- ...APIApprovals.PlatformSampleSettings.approved.txt | 13 +------------ .../APIApprovals.PublicClr.approved.txt | 2 +- 4 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 7ca0b549c0..914aa55f6c 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -1,16 +1,5 @@ { - "OnMessage": { - "Method": { - "Name": "<.ctor>b__0_0", - "AssemblyName": "ServiceControl.Audit, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null", - "ClassName": "ServiceControl.Audit.Infrastructure.Settings.Settings+<>c", - "Signature": "System.Threading.Tasks.Task <.ctor>b__0_0(System.String, System.Collections.Generic.Dictionary`2[System.String,System.String], Byte[], System.Func`1[System.Threading.Tasks.Task])", - "Signature2": "System.Threading.Tasks.Task <.ctor>b__0_0(System.String, System.Collections.Generic.Dictionary`2[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Byte[], System.Func`1[[System.Threading.Tasks.Task, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]])", - "MemberType": 8, - "GenericArguments": null - }, - "Target": {} - }, + "MessageFilter": null, "RunInMemory": false, "ValidateConfiguration": true, "DisableRavenDBPerformanceCounters": true, diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt index 25e7f66626..99458c9511 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt @@ -187,7 +187,7 @@ namespace ServiceControl.Audit.Infrastructure.Settings public string LicenseFileText { get; set; } public int MaxBodySizeToStore { get; set; } public int MaximumConcurrencyLevel { get; set; } - public System.Func, byte[], System.Func, System.Threading.Tasks.Task> OnMessage { get; set; } + public System.Func MessageFilter { get; set; } public int Port { get; set; } public bool PrintMetrics { get; } public string RootUrl { get; } diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index ee5ead1e4b..ea64ffed18 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -1,18 +1,7 @@ { "NotificationsFilter": null, "AllowMessageEditing": false, - "OnMessage": { - "Method": { - "Name": "<.ctor>b__0_0", - "AssemblyName": "ServiceControl, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null", - "ClassName": "ServiceBus.Management.Infrastructure.Settings.Settings+<>c", - "Signature": "System.Threading.Tasks.Task <.ctor>b__0_0(System.String, System.Collections.Generic.Dictionary`2[System.String,System.String], Byte[], System.Func`1[System.Threading.Tasks.Task])", - "Signature2": "System.Threading.Tasks.Task <.ctor>b__0_0(System.String, System.Collections.Generic.Dictionary`2[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Byte[], System.Func`1[[System.Threading.Tasks.Task, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]])", - "MemberType": 8, - "GenericArguments": null - }, - "Target": {} - }, + "MessageFilter": null, "RunInMemory": false, "EmailDropFolder": null, "ValidateConfiguration": true, diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt index 79bb5f6799..4e3640ef17 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PublicClr.approved.txt @@ -72,8 +72,8 @@ namespace ServiceBus.Management.Infrastructure.Settings public bool IngestErrorMessages { get; set; } public string LicenseFileText { get; set; } public int MaximumConcurrencyLevel { get; set; } + public System.Func MessageFilter { get; set; } public string NotificationsFilter { get; set; } - public System.Func, byte[], System.Func, System.Threading.Tasks.Task> OnMessage { get; set; } public int Port { get; set; } public bool PrintMetrics { get; } public System.TimeSpan ProcessRetryBatchesFrequency { get; set; }