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 Connecting to Redis via Unix Socket #58

Closed
dpipemazo opened this issue Aug 27, 2020 · 11 comments · Fixed by #59
Closed

Support Connecting to Redis via Unix Socket #58

dpipemazo opened this issue Aug 27, 2020 · 11 comments · Fixed by #59
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dpipemazo
Copy link
Contributor

Is your enhancement related to a problem? Please describe.

It would be nice to support connecting to and querying redis via a unix socket. We use this form of communicating with Redis frequently at Elementary since we run redis on the edge on our robots. We are currently spinning up our grafana server also on the edge for local development and it makes our docker-compose configuration cleaner if we don't have to map ports onto the host or assume container/service names.

Describe the solution you'd like

Allow for something like below:

Screenshot from 2020-08-27 12-03-26

Describe alternatives you've considered

We can (and currently do) support connection via TCP. When running grafana in its own container though the data source either needs to be:

  1. localhost:port, in which case we need to map the port from the redis container onto the host machine. This adds a potentially preventable port mapping
  2. container_name:port in which case we're hardcoding a dependency that our redis is launched with a certain container name which is not ideal.

Thanks for taking a look and apologies for filing so many issues in the past week or so. The grafana integration is really fantastic as-is and this would just make it a bit better for us. I do realize this is a fairly niche use case.

@mikhail-vl mikhail-vl added the enhancement New feature or request label Aug 27, 2020
@mikhail-vl
Copy link
Contributor

mikhail-vl commented Aug 27, 2020

@dpipemazo Redis Data Source based on Radix golang client (https://github.com/mediocregopher/radix). I don't see an option to connect using the Unix Socket at first sight. If it is supported by Radix, it would be ideal.

I can take a closer look later, but we can ask Radix developer if it's something he would be interested to implement. I see that some Redis clients support connecting using a Unix socket.

@dpipemazo
Copy link
Contributor Author

Yes, I agree it should come from the client. We've been using redis-py and hiredis via the unix socket for a while with good success, makes sense this support isn't in all of the language clients, particularly a language like golang which is really geared at networked connections. I'll file an issue over there. Thanks!

@mikhail-vl
Copy link
Contributor

@dpipemazo Thank you. Please put it here as well for tracking.

@dpipemazo
Copy link
Contributor Author

Issue filed on radix

@mikhail-vl
Copy link
Contributor

@dpipemazo Thank you for submitting the issue for radix. Looks like we found the way how we can add Socket connection as an additional client type.
I will test and let you know later tonight.

@mikhail-vl
Copy link
Contributor

@dpipemazo Let me add additional Client type for socket connection and will send you branch to try. Give me 15 minutes.

@dpipemazo
Copy link
Contributor Author

@mikhailredis thank you! This is very quick and much appreciated.

@mikhail-vl
Copy link
Contributor

@dpipemazo Please verify and let me know if it works for you: https://github.com/RedisTimeSeries/grafana-redis-datasource/tree/feat/socket

@dpipemazo
Copy link
Contributor Author

Thanks! Trying now 👍

@dpipemazo
Copy link
Contributor Author

Works great! 🎉

Screenshot from 2020-08-27 20-34-15

Thank you!!

@mikhail-vl mikhail-vl added this to the Version 1.2 milestone Aug 28, 2020
@mikhail-vl mikhail-vl self-assigned this Aug 28, 2020
@mikhail-vl
Copy link
Contributor

@dpipemazo Merged to master. Please let me know if there is anything else!

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

Successfully merging a pull request may close this issue.

2 participants