Fix key in cache instrumentation for Rails 7.2#81
Open
skipkayhil wants to merge 1 commit intoShopify:mainfrom
Open
Fix key in cache instrumentation for Rails 7.2#81skipkayhil wants to merge 1 commit intoShopify:mainfrom
skipkayhil wants to merge 1 commit intoShopify:mainfrom
Conversation
Previously, the key being logged in cache instrumentation would always be normalized by the instrumenter. However, many cache methods did not pass all of the options necessary to normalize a key into the instrumenter and so the key would not always reflect the actual key used in the cache. In rails/rails@9af99bf, the instrumenter was updated to no longer normalize the key because of this inconsistency. Now the caller of the instrument method is expected to pass the normalized key instead of the pre-normalized key. To maintain compatibility with Rails both before and after this commit, The key passed into instrument is conditional based on the Rails version, and the conditional can eventually be removed once Rails 7.2 is the minimum supported version Co-authored-by: Nick Schwaderer <nick.schwaderer@shopify.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the key being logged in cache instrumentation would always be normalized by the instrumenter. However, many cache methods did not pass all of the options necessary to normalize a key into the instrumenter and so the key would not always reflect the actual key used in the cache.
In rails/rails@9af99bf, the instrumenter was updated to no longer normalize the key because of this inconsistency. Now the caller of the instrument method is expected to pass the normalized key instead of the pre-normalized key.
To maintain compatibility with Rails both before and after this commit, The key passed into instrument is conditional based on the Rails version, and the conditional can eventually be removed once Rails 7.2 is the minimum supported version