search

endclothing / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
150 stars 77 forks source link

Replace eval with appropriate commands on the Redis storage adapter. #13

Closed marcos-sandim closed 2 years ago

marcos-sandim commented 4 years ago

The use of the eval command allow us to offload some work from the service collecting metrics to the Redis server where they are stored. But this becomes a problem when we have a Redis server shared among many services.

Another issue is that the conditional statements used in the Lua scripts were causing problems where the metadata was being left unset in some cases due to direct comparison of floating point values. The good news is that we don't really need these conditional statements. The HSET used to set the metadata can be replaced by HSETNX, that only sets the value if the key does not already exists. And the SADD command is idempotent, so there is no problem calling it multiple times with the same parameters.

As an improvement to consistency, we used Redis' transactions through MULTI and EXEC.

~~Since this changes are meant for versions pre 1.0.0 to support PHP versions pre 7.3, we also included the option to select the Redis database that was not available in v0.9.1.~~

NoelDavies commented 4 years ago

Please rebase this and it can be merged

martinssipenko commented 4 years ago

@marcos-sandim can you please update your branch to be up to date with master?

NoelDavies commented 4 years ago

Can you add additional (integration) tests for this just to check the new functionality please? Thanks