Allow incrementing counters with floats #22
Conversation
LKaemmerling
force-pushed
the
allow-incrementing-counters-with-floats
branch
from
7b9fea1 to
5c4ac50
Compare
rdohms
left a comment
rdohms
left a comment
There was a problem hiding this comment.
Small comment in case you want to improve understanding, but not blocking.
src/Prometheus/Storage/APC.php
Outdated
| while (!$done) { | ||
| $old = apcu_fetch($valueKey); | ||
| if ($old !== false) { | ||
| $done = apcu_cas($valueKey, $old, $this->toInteger($this->fromInteger($old) + $data['value'])); |
There was a problem hiding this comment.
am I correct in assuming the toInteger method is to avoid storing a float, and we then convert it back to float?
Might be worth noting that in the commit, or maybe renaming the method to be more expicit.
There was a problem hiding this comment.
Yep, apcu can only store integers. I was "confused" by this too a bit at the first view, but then I understood it. It packs the float value as its binary represent which is then an int. The methods was used before and i just reused it but yeah it makes sense to clarify it.
There was a problem hiding this comment.
@rdohms i renamed them, could you please have a look again?
Signed-off-by: Lukas Kämmerling <lukas.kaemmerling@hetzner-cloud.de>
Signed-off-by: Lukas Kämmerling <lukas.kaemmerling@hetzner-cloud.de>
Signed-off-by: Lukas Kämmerling <lukas.kaemmerling@hetzner-cloud.de>
LKaemmerling
force-pushed
the
allow-incrementing-counters-with-floats
branch
from
5c4ac50 to
7e3186e
Compare
This allows incrementing counters with floats.
For this, we need to change the storing logic within the
APCuStorage, which usedapcu_incbefore and now uses the same way as we have it for gauges. We should refactor this a bit in the future.Additionally, we need to set the Redis storage command based on what the type of the value is with that we want to increment, otherwise, it won't increment and fail "silently".
https://github.com/PromPHP/prometheus_client_php/pull/22/files#diff-e8fd41f0e9039fc21760030aeae90caae1f76729cac8d15cdb3a96df34c37454R45
Closes #20