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

Support for valkeys://localhost:6379 #952

Closed
FalkW opened this issue Sep 10, 2024 · 4 comments · Fixed by #959
Closed

Support for valkeys://localhost:6379 #952

FalkW opened this issue Sep 10, 2024 · 4 comments · Fixed by #959
Assignees
Labels

Comments

@FalkW
Copy link

FalkW commented Sep 10, 2024

Describe the problem
A clear and concise description of what the question is.

What version of redis_exporter are you running?
Please run redis_exporter --version if you're not sure what version you're running.
[ ] 1.62.0

Running the exporter
Running the exporter with default values of the bitnami/valkey helm chart

Additional context
Valkey replaced rediss://localhost:6379 with valkeys://localhost:6379 which causes the export no longer to scrape the metrics. See: bitnami/charts#29173 I'd assume the connection will remain being made via valkeys://.. in the future.

@oliver006
Copy link
Owner

Good question, I'd have to check if this needs to be handled in the downstream library (and if we can work around this temporarily with a strings.Replace() to get it to work)
I'm a little short on time right now but if you want to look into this it'd be much appreciated.

@zuiderkwast
Copy link

Valkey itself doesn't use a scheme for anything.

valkey-cli, the CLI tool shipped with Valkey, supports specifying connection parameters using redis:// and valkey://. It's fully backward compatible with Redis.

It's unfortunate if anyone thinks redis_exporter older versions don't work with Valkey. That's not true.

@oliver006
Copy link
Owner

It's unfortunate if anyone thinks redis_exporter older versions don't work with Valkey. That's not true.

Agree @zuiderkwast. Do you think there docs could be more clear? Anything you'd change? Happy to incorporate changes to highlight this.

Also, if you think there are additional tests that could help with making sure that redis_exporter stays compatible than let me know.

@zuiderkwast
Copy link

Hey Oliver. It's good that you support both schemes. I don't know about the docs. I just know that someone who saw the latest release notes about the valkey scheme assumed that it means valkey support in general. Not sure if you could have done anything about that. Maybe not.

I think it's a little surprising that bitnami charts started using a new scheme without checking that relevant tools support it. I'm don't know if we have promoted the valkey:// scheme at all. Long term, i guess it's right to use it though.

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

Successfully merging a pull request may close this issue.

3 participants