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

Call public get and set methods as part of cached and memoize decorators #417

Open
jennydale opened this issue Nov 28, 2022 · 3 comments
Open

Comments

@jennydale
Copy link

Cache's cached and memoize decorators should call the public self.get and self.set methods rather than internal self.cache.get and self.cache.set.

I am trying to use the ddtrace package to enable Datadog APM traces on flask-caching processes. ddtrace works by subclassing the Cache class and wrapping its methods in some tracing code. For many cache methods (get, set, add, etc) this works fine. However, Cache's cached and memoize decorators (which we use extensively in our project) are calling an internal self.cache.get() method rather than the public (subclassable) self.get() method, so ddtrace's tracing hooks aren't being exercised (and those flask-caching gets and sets aren't showing up in Datadog).

This problem is not solvable (as far as I can tell) without changes to Flask-Caching.

The default implementation of self.get is just to call self.cache.get (ditto for self.set), so for people not subclassing Cache there shouldn't be any difference.

Only downside to this proposal I can see is if someone has a subclass of Cache with their own implementations of get and set and they really don't want their custom stuff to get exercised when gets and sets are happening as part of the cached and memoize decorators. I'd guess more often than not people would want their custom versions to be called though, so this proposed change may well have benefits beyond ddtrace.

@pedrocavalero
Copy link

Hi @jennydale ! I'm evaluating using flask cache with datadog. Is this error still happening?

@jennydale
Copy link
Author

@pedrocavalero I haven't looked at it in over a year, but as far as I know this is still an issue. Good luck!

@pedrocavalero
Copy link

Thanks for your answer! I'm going to try it here. Regards!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants