redisotel: Missing server.address from sentinel traces

[AndrewWinterman]

We are using sentinel for high availability.

I'm in process of writing a library to support switchover from legacy deployments which do not use sentinel, and I noticed the server.address is missing from the sentinel spans:

Initialization of the tracer looks as follows:

	failoverOptions := &redis.FailoverOptions{
		SentinelAddrs:    []string{cfg.SentinelAddr},
		ClientName:       app.Info().PodID,
		DB:               cfg.DBNumber,
		Password:         string(cfg.Password),
		SentinelPassword: string(cfg.Password),

		// Base number of socket connections.
		// Default is 10 connections per every available CPU as reported by runtime.GOMAXPROCS.
		// If there is not enough connections in the pool, new connections will be allocated in excess of PoolSize,
		// you can limit it through MaxActiveConns
		PoolSize: cfg.PoolSize,
		// Minimum number of idle connections which is useful when establishing
		// new connection is slow.
		// Default is 0. the idle connections are not closed by default.
		MinIdleConns: cfg.MinIdleConns,
		// Maximum number of idle connections.
		// Default is 0. the idle connections are not closed by default.
		MaxIdleConns: cfg.MaxIdleConns,
		// Maximum number of connections allocated by the pool at a given time.
		// When zero, there is no limit on the number of connections in the pool.
		MaxActiveConns: cfg.MaxActiveConns,

		// there are additional idle
		ReplicaOnly:           role == RedisReplicaRoleReader,
		MasterName:            cfg.LeaderName,
		ContextTimeoutEnabled: true,

		// <<Stencil::Block(failoverOptions)>>

		// <</Stencil::Block>>
	}

	redis.SetLogger(&redisLogger{})

	var client redis.UniversalClient

	switch role {
	case RedisReplicaRoleReader:
		failoverOptions.RouteByLatency = cfg.RouteReadsByLatency
		client = redis.NewFailoverClusterClient(failoverOptions)
	case RedisReplicaRoleWriter:
		client = redis.NewFailoverClient(failoverOptions)
	default:
		return nil, fmt.Errorf("unknown RedisReplicaRole: %v", role)
	}

	ping := client.Ping(ctx)
	if ping.Err() != nil {
		return nil, errors.Wrapf(ping.Err(), "failed to contact Redis Sentinel at %q", cfg.SentinelAddr)
	}

	if err := redisotel.InstrumentTracing(client); err != nil {
		return nil, errors.Wrap(err, "failed to instrument Redis tracing")
	}
	if err := redisotel.InstrumentMetrics(client); err != nil {
		return nil, errors.Wrap(err, "failed to instrument Redis tracing")
	}
	return client, nil

Related:

• PR: add server address and port span attributes to redis otel trace instrumentation #2826
• Older issue adding it for standalone clients: redisotel: Missing server address and port attributes in spans #2825

Cause is because of this line here:

https://github.com/redis/go-redis/blob/master/sentinel.go#L90

It uses "FailoverClient" instead of any real address.

[AndrewWinterman]

What would the ramifications of simply returning the original or current leader address from the Options call be? https://github.com/redis/go-redis/blob/master/sentinel.go#L90

[ndyakov]

Hello @AndrewWinterman , thanks for reporting this. Will take the time next week to look more into it.

[ndyakov]

Actually, maybe @monkey92t or @vmihailenco can help you with this. I see it is related to redisotel and not go-redis