Skip to content

Make sure int keys are handled by redis integration #3130

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

Closed
antonpirker opened this issue Jun 4, 2024 · 2 comments · Fixed by #3132
Closed

Make sure int keys are handled by redis integration #3130

antonpirker opened this issue Jun 4, 2024 · 2 comments · Fixed by #3132
Assignees

Comments

@antonpirker
Copy link
Member

In this PR:
https://github.com/getsentry/sentry-python/pull/3073/files#diff-79a3788011179b24960642acf26b7cd5a7daaef510906fb683364309338546fcR47

there is probably a string.join() that lead to this error:
https://sentry.sentry.io/issues/5448730468/?project=1&query=release%3Abackend%40a13563880494efef0811369c3265d546508f2703&referrer=release-issue-stream

Make sure that _get_safe_command and _get_safe_key can handle integers.

@MartinBraquet
Copy link

MartinBraquet commented Jun 4, 2024

This issue has even more coverage than just int keys. It fails for any SCAN call in v2.4.0.

import redis
import sentry_sdk

sentry_sdk.init(dsn=...)

client = redis.Redis()
cursor, keys = client.scan()

Will raise

Traceback (most recent call last):
  File "_sentry_error.py", line 9, in <module>
    cursor, keys = client.scan()
  File "redis/commands/core.py", line 3026, in scan
    return self.execute_command("SCAN", *pieces, **kwargs)
  File "sentry_sdk/integrations/redis/_sync_common.py", line 72, in sentry_patched_execute_command
    cache_properties = _compile_cache_span_properties(
  File "sentry_sdk/integrations/redis/modules/caches.py", line 33, in _compile_cache_span_properties
    key_as_string = _key_as_string(key)
  File "sentry_sdk/integrations/redis/utils.py", line 61, in _key_as_string
    key = ", ".join(_safe_decode(x) for x in key)
TypeError: sequence item 0: expected str instance, int found

Most recent working version: v2.3.1.
Failing PR likely to be #3110
Thanks for your work

@sentrivana
Copy link
Contributor

Fix out now in 2.5.0

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

Successfully merging a pull request may close this issue.

5 participants