artprima / prometheus-metrics-bundle

Symfony 5/6 Prometheus Metrics Bundle
MIT License
127 stars 29 forks source link

`storage` does not allow resolving ENV parameters #96

Closed mvhirsch closed 5 months ago

mvhirsch commented 5 months ago

Hey :-)

we're using Symfony CLI and additionally added a password to our setup. This results in a custom environment variable, which needs to be resolved:

# config/services.yaml
parameters:
  env(REDIS_DSN): "%env(REDIS_SCHEME)%://%env(REDIS_PASSWORD)%@%env(REDIS_HOST)%:%env(int:REDIS_PORT)%"
  app.redis_dsn: '%env(resolve:REDIS_DSN)%'
# config/packages/prometheus_metrics.yaml
artprima_prometheus_metrics:
    storage: '%app.redis_dsn%'

Sadly it does not recognize the authentication string and fails connecting to redis. Looks like the Extension does not resolve env variables at all, just passing them to the Factory:

https://github.com/artprima/prometheus-metrics-bundle/blob/7a262f7e5deed3efad79ce91c75527caedafbb30/DependencyInjection/ArtprimaPrometheusMetricsExtension.php#L67-L75

Johnmeurt commented 5 months ago

Hello,

the extension is not supposed to resolve env vars, it is the container job. That's why factory layer has been added. What happened if you set your env var like this:

artprima_prometheus_metrics:
    storage: '%env(resolve:REDIS_DSN)%'
   # OR
   storage: "%env(REDIS_SCHEME)%://%env(REDIS_PASSWORD)%@%env(REDIS_HOST)%:%env(int:REDIS_PORT)%"
Johnmeurt commented 5 months ago

Hum, '%env(resolve:REDIS_DSN)%' will not match your parameter env(REDIS_DSN). It will result with the value of the env var REDIS_DSN in your parameter app.redis_dsn.

If you want to use env var named REDIS_DSN :

storage: '%env(REDIS_DSN)%'  # should be like this: "redis://%env(REDIS_PASSWORD)%@%env(REDIS_HOST)%:%env(int:REDIS_PORT)%"

Or simply use your redis env vars:

storage:
    type: redis
    host: %env(REDIS_HOST)%
    port: %env(int:REDIS_PORT)%
    password: %env(REDIS_PASSWORD)%
mvhirsch commented 5 months ago

Fun fact: FactoryRegistry parses the DSN but the "password"-part will be in $options['username'] . Using a workaround in the DSN solved my problem (notice the subtle : after scheme):

- env(REDIS_DSN): "%env(REDIS_SCHEME)%://%env(REDIS_PASSWORD)%@%env(REDIS_HOST)%:%env(int:REDIS_PORT)%"
+ env(REDIS_DSN): "%env(REDIS_SCHEME)%://:%env(REDIS_PASSWORD)%@%env(REDIS_HOST)%:%env(int:REDIS_PORT)%"

So it actually resolves it (as you've stated above) .

I don't have REDIS_HOST, REDIS_PORT, REDIS_PASSWORD variables on production. I use symfony CLI to concatenate them locally (while in develpment). On production I receive a ready-to-use REDIS_DSN.

I ran into a bigger problem since yesterday: this bundle (and the "unofficial prometheus client" it uses) does not support using a \RedisCluster nor Sentinel - which renders this bundle useless :sob:

Are there any plans to implement RedisCluster support?

Johnmeurt commented 5 months ago

I see. So nothing to do with your parameters I was not sure about my comment. This bundle can only provide features of promphp/prometheus_client_php. I m afraid you have to thumb up some awaiting PR and opened issues. If PR are merged, the configuration will be easy to implement.

mvhirsch commented 5 months ago

I see. Not sure if I (or one of my colleagues) will do. But if I do, I'll also send a PR to this project :+1:

In any case this can be closed.