Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AUTH info is not passed to the prometheus exporter, most metrics are not pulled #146

Open
doronl opened this issue Jul 14, 2022 · 2 comments

Comments

@doronl
Copy link

doronl commented Jul 14, 2022

You can see in the logs of the exporter if you try to pull the metrics:

curl nodeIP:9121/metrics

time="2022-07-13T10:38:43Z" level=info msg="Providing metrics at :9121/metrics"
time="2022-07-14T10:48:03Z" level=error msg="Couldn't set client name, err: NOAUTH Authentication required."
time="2022-07-14T10:48:03Z" level=error msg="Redis INFO err: NOAUTH Authentication required."

@voltbit @NataliAharoniPayu

@doronl
Copy link
Author

doronl commented Jul 14, 2022

was able to work this around by adding the below to the exoporter container, but still... IMO this should be in the example and in the helm if this is the solution...

      command:
      - "./redis_exporter"
      args:
      - "-redis.user=admin"
      - "-redis.password=adminpass"

@voltbit
Copy link
Contributor

voltbit commented Jul 14, 2022

Thanks for opening this issue @doronl , the ACL users in the repo are just for examples purposes. This is why the default unprotected user is also present. If no user is given a Redis client will try the default. The users of the operator should not use any of the ACL users present in this repo.

The operator itself uses two envvars that redis-cli itself is looking for:

https://github.com/PayU/redis-operator/blob/main/main.go#L34-L40
https://github.com/PayU/redis-operator/blob/main/helm/values.yaml#L37-L40
https://redis.io/docs/manual/cli/

Maybe we should add a dedicated user for the exporter just to make it clear that users need to do that if they remove the default user, so yes I think we should add the config mentioned in the issue.

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

No branches or pull requests

2 participants