#44 Metrics init draft #45
Conversation
EdgarSedov
force-pushed
the
metrics-initial-values
branch
5 times, most recently
from
ef6aecc
to
d4b1349
Compare
| * @return void | ||
| */ | ||
| public function wipeStorage(): void; | ||
| public function initHistogram(array $data): void; |
There was a problem hiding this comment.
Adding new methods to this interface is a breaking change, we should avoid doing it or maybe release this as a new version afterward (lesson learned from #43)
|
Hey @EdgarSedov, looks okay for me, but can you also please add tests? |
There was a problem hiding this comment.
Hey @EdgarSedov,
looks okay for me, but can you also please add tests?
Thanks! I guess 'll have some time a bit later this week, and will fix everything and add more tests
src/Prometheus/Storage/Redis.php
Outdated
| $metricKey = $this->toMetricKey($data); | ||
|
|
||
| // If hash exists, we skip init | ||
| if ($this->redis->hLen($metricKey)) { |
There was a problem hiding this comment.
The problem i can see here is what this condition blocks us from second init with another labels pool.
It can be made wiser, fetching data from __meta and comparing, but for now i just want to know if this concept is acceptable at all
| /** | ||
| * @param array[] $labelValuesSet | ||
| */ | ||
| public function init(array $labelValuesSet = []): void |
There was a problem hiding this comment.
Using such methods we can leave it to user to decide if he needs init or not, so there will be no changes in behaviour for regular usage
|
I haven't forgotten about this, just don't have enough time atm. Redis and InMemory are ready, only APC Adapter left |
EdgarSedov
force-pushed
the
metrics-initial-values
branch
from
d4b1349
to
a6ff731
Compare
|
Sorry, i'm not sure i can make enough time to code this properly (add support for all drivers, extend this for summaries, add labels "update" support after first init). If anyone could catch it or at least contribute implementation of missing drivers, that would be great. |
#44
Hi, i've made a working draft of these mechanics (currently only for redis). Please take a look.