Skip to content
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

fix(clickhouse): _sentry_span might be missing #3096

Merged
merged 2 commits into from
May 22, 2024

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented May 22, 2024

We started auto-enabling the ClickHouse integration in 2.0+. This led to it getting auto-enabled also for folks using ClickHouse with Django via django-clickhouse-backend, but it turns out that the integration doesn't work properly with django-clickhouse-backend and leads to AttributeError: 'Connection' object has no attribute '_sentry_span'.

This PR patches the integration to restore previous behavior -- i.e., the ClickHouse integration will not work with django-clickhouse-backend, but it will also not blow up.

Fixes #3088
Follow up in #3095


General Notes

Thank you for contributing to sentry-python!

Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.

For maintainers

Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.

Before running sensitive test suites, please carefully check the PR. Then, apply the Trigger: tests using secrets label. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

looking good!

@sentrivana sentrivana merged commit 38c14e9 into master May 22, 2024
111 checks passed
@sentrivana sentrivana deleted the ivana/clickhouse-no-attribute-sentry-span branch May 22, 2024 14:06
antonpirker added a commit that referenced this pull request May 23, 2024
* Some work to use same function for queries and caches module

* Moved functions to better place

* Added tests

* Fix

* Tests and linting

* Thats important for Python 3.6

* Fixed some tests

* Removed ipdb

* more fixing

* Cleanup

* Async cache spans

* Added async tests

* Fixed async tests

* Guard for not running async tests when there is no async fakeredis for that python version

* linting

* Use new names for ops

* Renamed for consistency

* fix _get_op()

* Cleaning up unused properties/parameters

* Use _get_safe_key in Django integration

* Fixed typing

* More tests

* Only return the keys in set_many, makes more sense

* Linting

* Cleanup

* fix(clickhouse): `_sentry_span` might be missing (#3096)

We started auto-enabling the ClickHouse integration in 2.0+. This led to it getting auto-enabled also for folks using ClickHouse with Django via `django-clickhouse-backend`, but it turns out that the integration doesn't work properly with `django-clickhouse-backend` and leads to `AttributeError: 'Connection' object has no attribute '_sentry_span'`.

* Make _get_safe_key work for all multi key methods in django and redis

* Fixed kwargs case and updated tests

* Updated tests

* cache.set should be cache.put

* Fix `cohere` testsuite for new release of `cohere`. (#3098)

* Check for new class to signal end of stream

---------

Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
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.

[Django] Clickhouse integration not working after upgrading from 1.45.0
2 participants