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
Resolve transaction trace aggregation race conditions #1166
Conversation
9e4924e
to
4d4a689
Compare
ee31a32
to
153878d
Compare
153878d
to
04c9233
Compare
src/Agent/NewRelic/Agent/Core/Aggregators/TransactionTraceAggregator.cs
Outdated
Show resolved
Hide resolved
Needs changelog entry |
src/Agent/NewRelic/Agent/Core/TransactionTraces/SlowestTransactionCollector.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a very good find. I especially like the additional unit tests that explicitly test the multithreaded scenarios. 🥇
Co-authored-by: Alex Hemsath <57361211+nr-ahemsath@users.noreply.github.com>
…on-trace-aggregator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional changes look good, approved again. 👍
Description
We have seen some test runs recently where the slowest transaction trace was not actually captured by the transaction trace aggregator. After reviewing the code there is a potential race condition where two transactions can be set as the 'slowest', but the actual slowest may not be retained due to a lack of locking.
This solution attempts to keep locking to a minimum, while ensuring we actually capture the slowest transaction.
Author Checklist
Reviewer Checklist