Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
61 changes: 61 additions & 0 deletions tests/Agent/UnitTests/CompositeTests/TransactionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.")
);
}
}
}