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

fix(config): server_url should return a valid URI for IPv6 hosts #194

Merged
merged 2 commits into from
May 2, 2024

Conversation

chen-anders
Copy link
Contributor

@chen-anders chen-anders commented May 2, 2024

Datadog's ddtrace-rb relies on the RedisClient::Config#server_url to return a valid redis:// address as part of initializing its tracing middleware.

https://github.com/DataDog/dd-trace-rb/blob/master/lib/datadog/tracing/contrib/redis/trace_middleware.rb#L79

With an IPv6 redis instance, sidekiq 7.2 would not start up due to bad URI(is not URI?): "redis://fd1b:ac4a:ab80::fcf:6379/12"which URI fails to parse properly.

This PR makes it so we try to detect an IPv6 address in the host and return a valid redis:// address in that particular case.

@byroot
Copy link
Member

byroot commented May 2, 2024

Thanks for the patch, it looks good, but could you add a regression test?

@chen-anders
Copy link
Contributor Author

@byroot - just added some more assertions to the server_url tests

@chen-anders
Copy link
Contributor Author

Can we rerun the Ruby 3.2 suite? Seems like something might've been flakey if tests passed on all other Ruby / redis versions.

@byroot byroot merged commit 382dda7 into redis-rb:master May 2, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants