fix: child span not acquiring transaction lock in some cases #1487
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Spans from the same transaction share the dropped spans map. If they end concurrency a race condition could happen when updating the map. Add a RW lock to prevent that.
dmathieu
approved these changes
Jul 11, 2023
marclop
reviewed
Jul 11, 2023
kruskall
changed the title
|
I've added a test and discovered more issues. It seems this is affecting more than just the dropped spans. I've pushed some changes and added a test. |
simitt
reviewed
Jul 12, 2023
Where should I add that ? Looking at past releases, we're adding all the changelog entries on release |
|
@simitt could you clarify the above question ? |
|
@kruskall in the past we have added entries to the Unreleased section. We've not been consistent about it though, and have sometimes just added the entries retrospectively at release time. Let's merge this and create a patch release - either adding the entry now, or creating the section at release time. |
|
Apologies @kruskall , I missed your message. |
marclop
approved these changes
Jul 26, 2023
This reverts commit 7fe3968.
The previous fix didn't cover all cases. The issue is deeper and affects more than the transaction data. We need to always acquire the tx lock before ending the span.
Co-authored-by: Marc Lopez Rubio <marc5.12@outlook.com>
Co-authored-by: Marc Lopez Rubio <marc5.12@outlook.com>
Co-authored-by: Marc Lopez Rubio <marc5.12@outlook.com>
* docs: update doc with go support policy * docs: remove go.mod comment the doc doesn't actually mention specific versions
…astic#1496) * docs: mention public bug bounty program in security policy * docs: remove SECURITY.md to fallback to org-wide security policy
kruskall
force-pushed
the
fix/dropped-span-race
branch
from
bd9e836
to
863c772
Compare
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Spans from the same transaction share the dropped spans map. If they end concurrency a race condition could happen when updating the map.
Add a RW lock to prevent that.
Closes #1484