Open
Description
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.
Metadata
Metadata
Assignees
Type
Projects
Milestone
Relationships
Development
No branches or pull requests
Activity
AndrewWinterman commentedon Mar 14, 2025
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#L90ndyakov commentedon Mar 20, 2025
Hello @AndrewWinterman , thanks for reporting this. Will take the time next week to look more into it.
[-]Missing server.address from sentinel traces[/-][+]redisotel: Missing server.address from sentinel traces[/+]ndyakov commentedon Mar 20, 2025
Actually, maybe @monkey92t or @vmihailenco can help you with this. I see it is related to
redisotel
and notgo-redis