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.

Sign up for GitHub

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

Jump to bottom

Support official redis cluster client #2523

Closed
wgiddens opened this issue Nov 21, 2023 · 1 comment · Fixed by #2394
Closed

Support official redis cluster client #2523

wgiddens opened this issue Nov 21, 2023 · 1 comment · Fixed by #2394
Labels
Enhancement New feature or request

Comments

@wgiddens
Copy link

Problem Statement

The Redis integration should support the redis-py RedisCluster client.

Sentry already supports redis-py-cluster, but it is end of life and has been ported into the official redis-py package, which Sentry also supports. It just doesn't support RedisCluster in redis-py.

Solution Brainstorm

It looks like @md384 already has a PR up for this: #2394

I couldn't find a corresponding issue so wanted to make sure it gets triaged.
@wgiddens wgiddens added the Enhancement New feature or request label Nov 21, 2023
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Nov 21, 2023
@wgiddens wgiddens mentioned this issue Nov 21, 2023
Merged
@sentrivana
Copy link
Contributor

Hey @wgiddens 👋🏻 Thanks for helping us make sure the PR doesn't fall through the cracks.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 Nov 22, 2023
@sentrivana sentrivana linked a pull request Nov 22, 2023 that will close this issue
feat(integrations): add support for cluster clients from redis sdk #2394
Merged
szokeasaurusrex added a commit that referenced this issue Dec 7, 2023
@md384 @szokeasaurusrex
feat(integrations): add support for cluster clients from redis sdk (#…
75f89b8 
…2394)

This change adds support for cluster clients from the redis sdk (as opposed to the rediscluster library).

This has also been tested in my own app which uses clusters (but not asyncio clusters).

Fixes GH-2523

* feat(integrations): add support for cluster clients from redis sdk

* fix: review round 1

* fix: explicit `is not None` checks

* fix: explicit `is not None` checks, take 2

* fix: add try/except to _set_db_data

* fix: handle additional spans and breadcrumbs caused by rediscluster initialization

* fix: typing for redis integration

* fix: simplify assertions

* add `capture_internal_exceptions`

Co-authored-by: Matthieu Devlin <matt@zumper.com>

* rerun CI

---------

Co-authored-by: Daniel Szoke <szokeasaurusrex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement New feature or request
Projects
GitHub Issues with 👀
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(integrations): add support for cluster clients from redis sdk md384/sentry-python
2 participants
@wgiddens @sentrivana