Skip to content

Comments

fix(vercel-ai): prevent tool call span map memory leak#19328

Open
lithdew wants to merge 1 commit intogetsentry:developfrom
lithdew:develop
Open

fix(vercel-ai): prevent tool call span map memory leak#19328
lithdew wants to merge 1 commit intogetsentry:developfrom
lithdew:develop

Conversation

@lithdew
Copy link

@lithdew lithdew commented Feb 14, 2026

Tool calls were stored in a global map and only cleaned up on tool errors, causing unbounded retention in tool-heavy apps (and potential OOMs when inputs/outputs were recorded). Store only span context in a bounded LRU cache and clean up on successful tool results; add tests for caching/eviction.

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
  • Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked.

Closes #issue_link_here

@lithdew
Copy link
Author

lithdew commented Feb 17, 2026

Any chance for a review? cc @nicohrubec @sergical @RulaKhaled

Copy link
Member

@nicohrubec nicohrubec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing! Generally looks good to me. The cleanup on successful tool results is definitely missing and there also seems to be no reason to store the full spans in the map. Do you think we really need the LRUCache? I get that the idea is to cap the amount of context we store, but I would rather not impose any arbitrary limits.

Would you like to work on this or should we take over?

@lithdew
Copy link
Author

lithdew commented Feb 19, 2026

The LRU cache isn't strictly necessary — it was just a safeguard to cap memory usage in case spans aren't cleaned up for some reason. Happy to drop it in favor of a plain map with proper cleanup on both success and error paths. I can make the change, or feel free to take it over — either works for me.

@nicohrubec
Copy link
Member

@lithdew If you could get rid of the LRUCache that would be great. I'll give it another look then

Tool calls were stored in a global map and only cleaned up on tool
errors, causing unbounded retention in tool-heavy apps (and potential
OOMs when inputs/outputs were recorded). Store only span context in a
bounded LRU cache and clean up on successful tool results; add tests for
caching/eviction.
@lithdew
Copy link
Author

lithdew commented Feb 19, 2026

Made the changes — would appreciate the re-review 🙏. cc @nicohrubec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants