feat(integrations): Add integration for aiomysql#4703
Open
tonal wants to merge 36 commits into
Open
Conversation
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
antonpirker
added
the
New Integration
antonpirker
changed the title
aiomysql
Contributor
|
Hey @tonal thanks for the PR, great work! Could you address the cursor comments above? |
antonpirker
changed the title
aiomysqlaiomysql
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Comment on lines
+2
to
+4
| """ | ||
| Adapted from module sentry_sdk.integrations.asyncpg | ||
| """ |
Contributor
There was a problem hiding this comment.
Hi @tonal ,
Thanks for the contribution, much appreciated!
Can you add unit tests for the integration as well? You can probably adapt them from the asyncpg tests.
Contributor
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Integrations
Other
Bug Fixes 🐛Anthropic
Pydantic Ai
Other
Internal Changes 🔧Litellm
Other
Other
🤖 This preview updates automatically when you update the PR. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Rewrite the aiomysql integration to fix issues raised in PR getsentry#4703: - Patch Cursor.execute and Cursor.executemany instead of Connection.query - Make _wrap_connect async (await inside span context) - Handle bytes/bytearray queries from executemany's internal batching - Normalize query text using " ".join(query.split()) for performance - Protect _sentry_skip_next_execute flag with try/finally to prevent leakage - Remove dead _wrap_cursor and unused _record context manager Add 17 end-to-end tests covering connect, execute, executemany, record_params, cursor iteration, connection pools, query source, span origin, and normalization. Update CI configuration: tox.ini envlist, GitHub Actions workflow with MySQL service container, and test suite config. Co-Authored-By: Qwen Code <noreply@anthropic.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Resolve merge conflicts in auto-generated files (tox.ini, package_dependencies.jsonl, releases.jsonl, CI workflows) by taking upstream versions and regenerating with aiomysql config. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Include fix for compromised transitive dependency (getsentry#6257). Regenerate tox.ini and CI workflows with aiomysql config. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace record_sql_queries with record_sql_queries_supporting_streaming to support span streaming (addresses sentry bot review comment). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Use set_attribute for StreamedSpan, set_data for regular Span - Branch _wrap_connect: traces.start_span with attributes when streaming, start_span with set_data otherwise (following asyncpg pattern) - Import StreamedSpan and has_span_streaming_enabled Addresses sentry bot and cursor bot review comments (HIGH severity). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…mocks - Call add_query_source inside context for StreamedSpan, outside for regular Span (following asyncpg pattern) - Fix tests to mock record_sql_queries_supporting_streaming instead of record_sql_queries Addresses cursor bot review comments. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
@alexander-alderman-webb Hi! I've resolved all the bot review comments (span streaming support, StreamedSpan compatibility, test mocks) and merged the latest master. The PR is ready for review when you have a chance. Thanks! |
Contributor
alexander-alderman-webb
left a comment
alexander-alderman-webb
left a comment
There was a problem hiding this comment.
Looks good me! Please resolve the conflicts in tox.ini as well and then I'd approve and merge
Contributor
|
I also just ran our checks and the lint and the new tests are failing. These block merging the PR. You can run the linter locally using the following command: ruff format --check tests sentry_sdk |
MySQL 8.0 uses caching_sha2_password which requires the cryptography package. Add it to tox config deps and regenerate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wait until the integration is tested over a few versions before auto-enabling. Also add cryptography dep for MySQL 8.0 auth. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolve conflicts in auto-generated files, add cryptography dep, remove from auto-enabling list, fix lint. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Upstream renamed record_sql_queries_supporting_streaming back to record_sql_queries since it now natively supports streaming. Update imports and mock targets accordingly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add import-not-found to type: ignore comments for aiomysql imports - Explicitly type span_attributes as dict[str, Any] for mypy - Remove accidentally committed requirements-dev.in/.txt Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
import-untyped is redundant when import-not-found covers the case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Looks like you pulled in an incompatibility between Pydantic AI and our instrumentation by updating the test matrix. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db77a50. Configure here.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Create _get_connect_data() to build breadcrumb dict directly from connection, instead of relying on span._data which is empty for NoOpSpan. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Add support for aiomysql to the SDK.