http.send: Add metric for counting inter-query cache hits #4087
Conversation
There was a problem hiding this comment.
👏 That's a great first contribution. 🎉
I've got one nitpick wrt the tests added, but that aside it's good to go.
Except for one more thing, the commit message's sign-off line has an email that doesn't look correct:
Signed-off-by: Mira Yadav <mirayadav@Miras-MacBook-Pro.local>
Could you have another look? LMK if you need assistance adjusting the commits.
topdown/http_test.go
Outdated
| @@ -2494,6 +2496,28 @@ func TestHTTPSendMetrics(t *testing.T) { | |||
| if m.Timer(httpSendLatencyMetricKey).Int64() == 0 { | |||
| t.Fatal("expected non-zero value for http.send latency metric") | |||
| } | |||
|
|
|||
| // add an inter-query cache | |||
There was a problem hiding this comment.
[nit] Perhaps we could wrap this new part in
t.Run("cache hits", func(t *testing.T) {
// ...
})and the existing code, beginning in line 2488, in
t.Run("latency", func(t *testing.T) {
// ...
}it'll help avoiding that the test function gets too unwieldy over time
|
Thank you for the quick review! I was able to edit my previous commit message to include the correct signoff, but this seems to have brought in all of the changes after that commit as new commits in this PR. Do you know how to fix this? |
|
What I'd do, I think, is, with your PR branch checked out and editor will appear. It in, select all the commits that should be here (your commits!), and remove all the lines of other commits. Then, change "pick" to "squash" for all commits except the first one of the remaining list, i.e., your commits you want to keep. Safe and close the buffer. Another editor will come up, showing you all the commits' messages, and you can edit them, combine them into one, with one (correct) signed-off-by line. (Happy to help you further, jump on a call, or something to get this sorted, but I've got to leave now. Timezones might make this difficult to do in real time.) |
mirayadav
force-pushed
the
interquery-cache-hits
branch
2 times, most recently
from
cc8061d
to
a2da06a
Compare
An inter-query cache is used to cache responses for http.send. This change allows users to gain insight into how the cache is performing by tracking the number of cache hits. Fixes: open-policy-agent#4023 Signed-off-by: Mira Yadav <mira.yadav620@gmail.com>
mirayadav
force-pushed
the
interquery-cache-hits
branch
from
a2da06a
to
66587da
Compare
|
Sign-off is fixed now, thanks for the information! |
An inter-query cache is used to cache responses for http.send. This change allows users to gain insight into how the cache is performing by tracking the number of cache hits.
Fixes: #4023
Signed-off-by: Mira Yadav mira.yadav620@gmail.com