Include option for single URL (dsn) for redis configuration

[pmcgoverncw]

The current configuration options for this bundle only allow for separate data points for a redis connection (host, port, username, password, etc.). Given that popular redis-related bundles (ex: snc/SncRedisBundle & predis/predis) allow for single URL redis connection definition, it makes sense to allow that in this bundle as well. This removes the necessity to have separate env vars to define redis connection parameters in two different ways (dsn vs. separate values) Furthermore, integrations with popular redis bundles would be pertinent here, i.e. allowing definition of an existing service to use for a redis connection (ex: reference cache.adapter.redis as the redis provider as an alternative to providing any connection params)

[denisvmedia]

Thank you for your input. At the moment I don't have plans adding this feature. But I'm always open to good PRs.

[pmcgoverncw]

I'm seeing now that this is first an issue with PromPHP/prometheus_client_php as that bundle, included in this bundle, does not support using a redis dsn over separate connection params either.

[denisvmedia]

Well, yes, but I don't see a problem with parsing DSN (in the end it's just a URI in a form of redis[s]://[pass@][ip|host|socket[:port]][/db-index]).

[pmcgoverncw]

I suggested in the other bundle that either predis/predis be implemented as the redis handler or to implement a similar URL parsing method to what predis/predis uses: https://github.com/predis/predis/blob/main/src/Connection/Parameters.php#L69

[pmcgoverncw]

Another example is the snc/sncredisbundle dns parser: https://github.com/snc/SncRedisBundle/blob/master/DependencyInjection/Configuration/RedisDsn.php#L169

[denisvmedia]

Thanks for your research. predis way seems to be the straight-forward one. Considering that in php we have a parse_url function, I'm not sure why SncRedisBundle doesn't use it.

[pmcgoverncw]

Do you have a defined docker container used to run the tests for this package (incl. web servicer to make /metrics/prometheus accessible)?

[denisvmedia]

I don't. But you may want to check how I run integration tests here. And unit+functional tests

[pmcgoverncw]

I saw those, but don't have a way to run GitHub actions locally - I've set up a basic dockerfile & docker-compose.yml with everything needed and will delete it before pushing my branch

[pmcgoverncw]

The work to require additional packages & copy the .integration/symfony files seems non-standard. going to be a lot of do & undo if this branch needs any revisions - I may have to push off any additional work on this until I have more free time. How are you running all of your tests locally?

[denisvmedia]

I run only functional and unit tests locally (for which I have redis running). I don't run the integration tests on my machine. Yes, it's not a standard approach, but I'm not aware of any better way to test the bundle against different symfony versions. We can discuss a better way if you have any ideas on that.

[denisvmedia]

Closing as stale. Feel free to re-open it, if you decide to proceed with the issue.

[tourze]

If someone want to use snc_redis in this bundle, can try this:

1. Create a new CollectorRegistry, like:

<?php

namespace App\Monitor;

use Prometheus\Storage\Adapter;
use Prometheus\Storage\Redis;

class CollectorRegistry extends \Prometheus\CollectorRegistry
{

    /**
     * 保留第一个参数的注入，兼容旧的用法
     *
     * @param Adapter $storageAdapter
     * @param Redis $redisStorage
     * @param bool $registerDefaultMetrics
     */
    public function __construct(Adapter $storageAdapter, Redis $redisStorage, bool $registerDefaultMetrics = true)
    {
        parent::__construct($redisStorage, $registerDefaultMetrics);
    }
}

2. Add to services.yaml:

  prometheus_metrics_bundle.custom.redis.adapter:
    class: Prometheus\Storage\Redis
    public: true
    factory: ['Prometheus\Storage\Redis', 'fromExistingConnection']
    arguments: ['@snc_redis.monitor']
  prometheus_metrics_bundle.collector_registry:
    class: App\Monitor\CollectorRegistry
    public: true
    arguments:
      - '@prometheus_metrics_bundle.adapter'
      - '@prometheus_metrics_bundle.custom.redis.adapter'

snc_redis.monitor is the service id that you want to inject, change whatever you want.

After that, prometheus-metrics-bundle will use snc_redis to connect redis service.