The problem/use-case that the feature addresses

Redis is only writing the log/console today the Port and not the Bind IP. It's needed especially in cases when the server starts with --bind arguments. e.g. redis-server --bind 1.1.1.1

Description of the feature

Add the bind IP

           _.-``__ ''-._                                             
      _.-``    `.  `_.  ''-._           Redis 6.0.8 (00000000/0) 64 bit
  .-`` .-```.  ```\/    _.,_ ''-._                                   
 (    '      ,       .-`  | `,    )     Running in standalone mode
 |`-._`-...-` __...-.``-._|'` _.-'|     Port: 6379
 |    `-._   `._    /     _.-'    |     PID: 107914
  `-._    `-._  `-./  _.-'    _.-'      Bind: 1.1.1.1                                 
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |           http://redis.io        
  `-._    `-._`-.__.-'_.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |                                  
  `-._    `-._`-.__.-'_.-'    _.-'                                   
      `-._    `-.__.-'    _.-'                                       
          `-._        _.-'                                           
              `-.__.-'            

Comment From: oranagra

While it makes some print the port (we always know it), redis doesn't always know which addresses it listens on (there could be many). if you used --bind then we know it, or if you used replica-announce-ip and such, but many other cases it binds on 0.0.0.0. i suppose printing 0.0.0.0 is not what you're looking for.

The way i see it, this log prints is just a friendly tip for a user working with the console (not the log file), to know what to pass to redis-cli.

i'm not sure what's the use case here? is it postmortem investigation? maybe we need to print other configs to the log file? or is it something that wants to parse the log file while redis is running? (in which case maybe it's better that whoever spawn up redis, will run echo ifconfig into the log file before running redis.

Comment From: gkorland

if you used --bind then we know it, or if you used replica-announce-ip

That is our case, so printing it out should work for us

But many other cases it binds on 0.0.0.0. i suppose printing 0.0.0.0 is not what you're looking for.

Correct that won't help in our case. Can't Redis know which external IP is it really using?

(in which case maybe it's better that whoever spawn up redis, will run echo ifconfig into the log file before running redis.

That can be nice too, I saw it many products that they log the configuration on startup, or when launched with a --log-config flag or something similar

Comment From: gkorland

i'm not sure what's the use case here?

The use case here, is for auto discovery of Redis servers deployed on unknown IPs

Comment From: oranagra

The use case here, is for auto discovery of Redis servers deployed on unknown IPs

if it was executed with an explicit --bind, then the IP is known. also processing the log file smells like a bad idea for auto discovery. seems like whoever executes it with --bind should log the ip address somewhere where the discovery mechanism can get it more easily (not the possibly pre-existing huge log file)

Comment From: Spartee

I believe @gkorland is referring to our (and our clients) use case here. We use Redis/RedisAI heavily.

We do not know the IP ahead of time as the mechanism we use to launch workloads (with Redis) picks the location based on the constraints specified by the user of our application (GPU/CPU, cores, threads, memory, etc). The method by which the scheduler chooses IP addresses is opaque to our application in most cases. Kubernetes is an exception here.

in which case maybe it's better that whoever spawn up redis, will run echo ifconfig into the log file before running redis.

Unfortunately this approach falls short. We have to use a launch binary that invokes the scheduler. If we launch echo ifconfig and redis-server sequentially (e.g. with ; syntax), they get placed on different compute nodes.

also processing the log file smells like a bad idea for auto discovery.

Agreed. parsing is not the strategy I would like to go with, however, due to the aforementioned constraints, we have not yet found a better strategy. (happy to list out the other methods we've tried) Our customers usually have parallel filesystems (lustre) so this doesn't affect our application as much as it would in other places. In addition to this being a needed feature for us and our clients, I believe that other users will appreciate this feature being available for debugging purposes as mentioned. I like the idea of an optional flag.

I have a local fork that has this feature implemented, but the way Guy specified it is much cleaner.

   // in server.c -> initserver

    char hostbuffer[256];
    char *IPbuffer;
    struct hostent *host_entry;
    int hostname;

    hostname = gethostname(hostbuffer, sizeof(hostbuffer));

    // checks /etc/hosts (needs error handling for if hostname isn't present)
    host_entry = gethostbyname(hostbuffer);

    IPbuffer = inet_ntoa(*((struct in_addr*)
                           host_entry->h_addr_list[0]));

    // warning so it always gets printed to log
    serverLog(LL_WARNING, "Hostname: %s", hostbuffer);
    serverLog(LL_WARNING, "IP: %s", IPbuffer)

Comment From: yossigo

@Spartee Thanks for clarifying this.

This is a very specific use case that is not relevant for most users. On top of that, we're likely to produce partial/incorrect information in many cases (think IPv6, multi-homed systems, networking that involves NAT, etc.). Doing hostname resolution works in your case - but it's not a general solution (there really isn't one).

Your code can be wrapped into a simple module though --

#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>

#include "redismodule.h"

int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
    char hostbuffer[256];
    char *IPbuffer;
    struct hostent *host_entry;
    int hostname;

    if (RedisModule_Init(ctx,"announceip",1,REDISMODULE_APIVER_1)
            == REDISMODULE_ERR) return REDISMODULE_ERR;

    hostname = gethostname(hostbuffer, sizeof(hostbuffer));

    // checks /etc/hosts (needs error handling for if hostname isn't present)
    host_entry = gethostbyname(hostbuffer);

    IPbuffer = inet_ntoa(*((struct in_addr*)
                           host_entry->h_addr_list[0]));

    RedisModule_Log(ctx, "warning", "Hostname: %s", hostbuffer);
    RedisModule_Log(ctx, "warning", "IP: %s", IPbuffer);

    return REDISMODULE_OK;
}

You can then load the module on startup using --loadmodule (or configuration file):

$ ./src/redis-server --loadmodule `pwd`/announceip.so 

And get the output:

9143:M 04 Mar 2021 14:59:03.515 # <announceip> Hostname: ubuntu
9143:M 04 Mar 2021 14:59:03.515 # <announceip> IP: 127.0.1.1

Comment From: Spartee

@yossigo This looks like a great solution and should work well for us. We had considered the module approach but thought it might be overkill. The way you laid it out looks very simple. I will try it out and get back to y'all in this issue soon.

Assuming this works, I am happy to maintain this as a Redis Module for others who would like this solution. Although, like you said, our use cases are somewhat niche and we can appreciate your stance on that.

Comment From: Spartee

Done! Works like a charm.

Module will continue to be fleshed out with error handling and build handlers for other systems, but for now the module will be at https://github.com/Spartee/RedisIP

Thank you for all the help! @gkorland @DvirDukhan @yossigo @oranagra As far as our side is concerned, this issue can be closed.