PartialSuccessTransmissionPolicy: Avoid unnecessary content deseriali…

…zations (#2500) * PartialSuccessTransmissionPolicy : Do not deserialize content unless absolutely needed * Updated changelog.md * PartialSuccessTransmissionPolicy : Do not deserialize content unless absolutely needed * Updated changelog.md * Only deserialize the initial transmission once * retrigger checks * Move changelog entry to vnext section * retrigger checks * retrigger checks * retrigger checks Co-authored-by: Timothy Mothra <tilee@microsoft.com>
microsoft · Dec 21, 2021 · 4b0a419 · 4b0a419
1 parent 11b2dae
commit 4b0a419
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 9 deletions.
diff --git a/...ryChannel.Tests/Implementation/TransmissionPolicy/PartialSuccessTransmissionPolicyTest.cs b/...ryChannel.Tests/Implementation/TransmissionPolicy/PartialSuccessTransmissionPolicyTest.cs
@@ -241,6 +241,40 @@ public void IfIndexMoreThanNumberOfItemsInTransmissionIgnoreError()
             Assert.AreEqual(0, enqueuedTransmissions.Count);
         }
 
+        [TestMethod]
+        public void IfIndexNegativeIgnoreError()
+        {
+            IList<Transmission> enqueuedTransmissions = new List<Transmission>();
+            var transmitter = new StubTransmitter
+            {
+                OnEnqueue = t => { enqueuedTransmissions.Add(t); }
+            };
+
+            var policy = new PartialSuccessTransmissionPolicy();
+            policy.Initialize(transmitter);
+
+            var items = new List<ITelemetry> { new EventTelemetry() };
+            Transmission transmission = new Transmission(new Uri("http://uri"), items, "type", "encoding");
+
+            // Index is 0-based
+            string response = BackendResponseHelper.CreateBackendResponse(
+                itemsReceived: 2,
+                itemsAccepted: 1,
+                errorCodes: new[] { "408" },
+                indexStartWith: -1);
+
+            var wrapper = new HttpWebResponseWrapper
+            {
+                StatusCode = 206,
+                Content = response
+            };
+
+            transmitter.OnTransmissionSent(new TransmissionProcessedEventArgs(transmission, null, wrapper));
+
+            Assert.AreEqual(0, transmitter.BackoffLogicManager.ConsecutiveErrors);
+            Assert.AreEqual(0, enqueuedTransmissions.Count);
+        }
+
         [TestMethod]
         public void DoesNotSendTransmissionForUnsupportedCodes()
         {
@@ -259,7 +293,7 @@ public void DoesNotSendTransmissionForUnsupportedCodes()
             string response = BackendResponseHelper.CreateBackendResponse(
                 itemsReceived: 50,
                 itemsAccepted: 45,
-                errorCodes: new[] { "400", "402", "502", "409", "417" });
+                errorCodes: new[] { "400", "402", "502", "409", "417", "206" });
 
             var wrapper = new HttpWebResponseWrapper
             {

diff --git a/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs b/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs
@@ -67,7 +67,7 @@ public void BreezeResponseWasNotParsedWarning(string exception, string response,
             this.WriteEvent(5, exception ?? string.Empty, response ?? string.Empty, this.ApplicationName);
         }
 
-        [Event(6, Message = "Unexpected backend response. Items # in batch {0} >= Error index in response: {1}.", Level = EventLevel.Warning)]
+        [Event(6, Message = "Unexpected backend response. Items # in batch {0} <= Error index in response: {1}.", Level = EventLevel.Warning)]
         public void UnexpectedBreezeResponseWarning(int size, int index, string appDomainName = "Incorrect")
         {
             this.WriteEvent(6, size, index, this.ApplicationName);
@@ -575,6 +575,12 @@ public void AuthenticationPolicyCaughtFailedIngestion(string transmissionId, str
             this.WriteEvent(78, transmissionId ?? string.Empty, statusCode ?? string.Empty, statusDescription ?? string.Empty, this.ApplicationName);
         }
 
+        [Event(79, Message = "Unexpected backend response. Invalid Error index in response: {0}.", Level = EventLevel.Warning)]
+        public void UnexpectedBreezeResponseErrorIndexWarning(int index, string appDomainName = "Incorrect")
+        {
+            this.WriteEvent(79, index, this.ApplicationName);
+        }
+
         private static string GetApplicationName()
         {
             //// We want to add application name to all events BUT

diff --git a/...verTelemetryChannel/Implementation/TransmissionPolicy/PartialSuccessTransmissionPolicy.cs b/...verTelemetryChannel/Implementation/TransmissionPolicy/PartialSuccessTransmissionPolicy.cs
@@ -48,17 +48,15 @@ private static string ParsePartialSuccessResponse(Transmission initialTransmissi
             string newTransmissions = null;
             if (backendResponse.ItemsAccepted != backendResponse.ItemsReceived)
             {
-                string[] items = JsonSerializer
-                    .Deserialize(initialTransmission.Content)
-                    .Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
+                string[] initialTransmissionItems = null;
 
                 foreach (var error in backendResponse.Errors)
                 {
                     if (error != null)
                     {
-                        if (error.Index >= items.Length || error.Index < 0)
+                        if (error.Index < 0)
                         {
-                            TelemetryChannelEventSource.Log.UnexpectedBreezeResponseWarning(items.Length, error.Index);
+                            TelemetryChannelEventSource.Log.UnexpectedBreezeResponseErrorIndexWarning(error.Index);
                             continue;
                         }
 
@@ -70,13 +68,27 @@ private static string ParsePartialSuccessResponse(Transmission initialTransmissi
                             error.StatusCode == ResponseStatusCodes.ResponseCodeTooManyRequests ||
                             error.StatusCode == ResponseStatusCodes.ResponseCodeTooManyRequestsOverExtendedTime)
                         {
+                            // Deserialize the initial transmission content if it has not been deserialized yet
+                            if (initialTransmissionItems == null)
+                            {
+                                initialTransmissionItems = JsonSerializer
+                                .Deserialize(initialTransmission.Content)
+                                .Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
+                            }
+
+                            if (error.Index >= initialTransmissionItems.Length)
+                            {
+                                TelemetryChannelEventSource.Log.UnexpectedBreezeResponseWarning(initialTransmissionItems.Length, error.Index);
+                                continue;
+                            }
+
                             if (string.IsNullOrEmpty(newTransmissions))
                             {
-                                newTransmissions = items[error.Index];
+                                newTransmissions = initialTransmissionItems[error.Index];
                             }
                             else
                             {
-                                newTransmissions += Environment.NewLine + items[error.Index];
+                                newTransmissions += Environment.NewLine + initialTransmissionItems[error.Index];
                             }
 
                             // Last status code is used for tracing purposes. We will get only one out of many. Most likely they all will be the same.

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## VNext
 - [Validate exception stack trace line numbers to comply with endpoint restrictions.](https://github.com/microsoft/ApplicationInsights-dotnet/issues/2482)
+- [Removed redundant memory allocations and processing in PartialSuccessTransmissionPolicy for ingestion sampling cases](https://github.com/microsoft/ApplicationInsights-dotnet/issues/2445)
 
 ## Version 2.20
 - [Allow Control of Documents sampling for QuickPulse telemetry](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2425)