fix: Clearing transaction context for held transactions and async WCF client instrumentation timing. (#1608)

Author: nrcventura

src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs

@@ -133,6 +133,11 @@ public void End(bool captureResponseTime = true)
                 {
                     LogFinest("Response time captured.");
                 }
+
+                // Once the response time is captured, the only work remaining for the transaction can be
+                // async work that is holding onto the transaction. We need to remove the transaction
+                // from the transaction context so that it will not be reused.
+                Agent._transactionService.RemoveOutstandingInternalTransactions(true, true);
             }
 
             var remainingUnitsOfWork = NoticeUnitOfWorkEnds();

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Wcf3/ServiceChannelProxyWrapper.cs

@@ -124,7 +124,7 @@ void OnSuccess(System.Runtime.Remoting.Messaging.IMethodReturnMessage methodRetu
                     segment.RemoveSegmentFromCallStack();
                     transaction.Hold();
                     var task = (Task)methodReturnMessage.ReturnValue;
-                    task.ContinueWith(ContinueWork);
+                    task.ContinueWith(ContinueWork, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.HideScheduler);
 
                     void ContinueWork(Task t)
                     {

tests/Agent/UnitTests/CompositeTests/TransactionTests.cs

@@ -4,6 +4,7 @@
 using NewRelic.Agent.Api;
 using NewRelic.Agent.Core;
 using NewRelic.Agent.Core.Attributes;
+using NewRelic.Agent.Core.Transactions;
 using NewRelic.Agent.Extensions.Providers.Wrapper;
 using NewRelic.Agent.TestUtilities;
 using NewRelic.Testing.Assertions;
@@ -676,5 +677,65 @@ public void ResponseTimeShouldOnlyBeCapturedOnce()
             var timingMetric = _compositeTestAgent.Metrics.First(x => x.MetricName.Name == "WebTransaction");
             Assert.LessOrEqual(timingMetric.Data.Value1, upperBoundStopWatch.Elapsed.TotalSeconds);
         }
+
+        [Test]
+        public void TransactionShouldBeRemovedFromContextStorageWhenResponseTimeIsCaptured()
+        {
+            // Setup a transaction where fire and forget async work is tracked
+            var transaction = _agent.CreateTransaction(
+                isWeb: false,
+                category: "category",
+                transactionDisplayName: "transactionName",
+                doNotTrackAsUnitOfWork: true);
+            var segment = _agent.StartTransactionSegmentOrThrow("fireAndForgetSegment");
+            transaction.Hold();
+            // Response time should be tracked and the current transaction removed from storage.
+            transaction.End();
+
+            var transactionAfterCallToEnd = _agent.CurrentTransaction;
+
+            // The fire and forget segment ends and then releases the transaction
+            segment.End();
+            transaction.Release();
+
+            _compositeTestAgent.Harvest();
+
+            //Use the OtherTransaction/all metric to confirm that the transaction was transformed and harvested
+            var timingMetric = _compositeTestAgent.Metrics.First(x => x.MetricName.Name == "OtherTransaction/all");
+            NrAssert.Multiple(
+                    () => Assert.IsFalse(transactionAfterCallToEnd.IsValid, "The current transaction should be the NoOpTransaction."),
+                    () => Assert.AreEqual(1.0, timingMetric.Data.Value0, "The transaction should be harvested.")
+                );
+        }
+
+        [Test]
+        public void TransactionShouldNotBeRemovedFromContextStorageWhenResponseTimeIsNotCaptured()
+        {
+            // Setup a transaction where fire and forget async work is tracked
+            var transaction = _agent.CreateTransaction(
+                isWeb: false,
+                category: "category",
+                transactionDisplayName: "transactionName",
+                doNotTrackAsUnitOfWork: true);
+            var segment = _agent.StartTransactionSegmentOrThrow("fireAndForgetSegment");
+            transaction.Hold();
+            // Response time should be tracked and the current transaction removed from storage.
+            transaction.End(captureResponseTime: false);
+
+            var transactionAfterCallToEnd = _agent.CurrentTransaction;
+
+            // The fire and forget segment ends and then releases the transaction
+            segment.End();
+            transaction.Release();
+
+            _compositeTestAgent.Harvest();
+
+            //Use the OtherTransaction/all metric to confirm that the transaction was transformed and harvested
+            var timingMetric = _compositeTestAgent.Metrics.First(x => x.MetricName.Name == "OtherTransaction/all");
+            NrAssert.Multiple(
+                    () => Assert.IsTrue(transactionAfterCallToEnd.IsValid, "The current transaction should not be the NoOpTransaction."),
+                    () => Assert.AreEqual(1.0, timingMetric.Data.Value0, "The transaction should be harvested.")
+                );
+        }
     }
 }