-
Notifications
You must be signed in to change notification settings - Fork 511
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
Add cache.hit
and cache.item_size
to Django
#2057
Conversation
…try/sentry-python into antonpirker/2403-cache-hit-django
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong return value
Co-authored-by: Neel Shah <[email protected]>
with hub.start_span(op=OP.CACHE, description=description) as span: | ||
value = original_method(*args, **kwargs) | ||
|
||
if value: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antonpirker you can't call __bool__
of the returned value, it might not be implemented. You should check if a default value was returned, in which case it is a cache miss, or if there is not default value configured you should check value is not None
.
I recently reported the same issue on ddtrace
, you can check of examples on how to reproduce there DataDog/dd-trace-py#5460 and here is how they handle it DataDog/dd-trace-py#5477
In Django we want to add information to spans if a configured cache was hit or missed and if hit what the item_size of the object in the cache was.
Fixes #2043