Skip to content
This repository has been archived by the owner on Oct 22, 2019. It is now read-only.

Flush only metric data from Redis instead of complete server #81

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jlis
Copy link

@jlis jlis commented Jun 12, 2018

How it currently behaves:
After the flushRedis() method is called, the complete Redis server incl. all databases is flushed using FLUSHALL. One dedicated Redis server/cluster is required to use the Redis adapter.

How it should behave:
Only the metric data (the keys and the actual metrics) are deleted via a LUA script. A shared Redis server/cluster can be used for collecting metrics using the Redis adapter.

I also switched from PHPUnit 4.1.0 to ~5.7 since >=PHP 5.6.3 is required in the composer file.

@jlis
Copy link
Author

jlis commented Jul 18, 2018

ping.

@seiffert
Copy link
Contributor

seiffert commented Aug 6, 2018

Hi @jlis, thank you for your contribution! Would you mind adding a test case that validates that no other data is removed from the Redis instance? Thanks!

@jlis
Copy link
Author

jlis commented Aug 9, 2018

@seiffert I added the tests as requested :)

* @param $job
* @param $groupingKey
* @param $method
*/
private function doRequest(CollectorRegistry $collectorRegistry, $job, $groupingKey, $method)
private function doRequest(CollectorRegistry $collectorRegistry = null, $job, $groupingKey, $method)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What argument errors does this prevent? When do they occur?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the blackbox tests, I'm getting an argument error/notice since the CollectorRegistry is expected but null is given in PushGateway::delete() method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #82.

There was 1 error:

1) Test\BlackBoxPushGatewayTest::pushGatewayShouldWork
Argument 1 passed to Prometheus\PushGateway::doRequest() must be an instance of Prometheus\CollectorRegistry, null given, called in /var/www/html/src/Prometheus/PushGateway.php on line 54 and defined

/var/www/html/src/Prometheus/PushGateway.php:63
/var/www/html/src/Prometheus/PushGateway.php:54
/var/www/html/tests/Test/BlackBoxPushGatewayTest.php:36

@jlis
Copy link
Author

jlis commented Nov 7, 2018

Ping

@dbaltas
Copy link

dbaltas commented Feb 15, 2019

any news on this one?

@jlis
Copy link
Author

jlis commented Feb 15, 2019

Nah, I guess the repo is dead.

@NoelDavies
Copy link

This project is dead, but I'm maintaining it under my employer - https://github.com/endclothing/prometheus_client_php. Feel free to submit the PR there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants