redis / go-redis

Redis Go client
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
19.81k stars 2.34k forks source link

redisotel: Missing server address and port attributes in spans #2825

Closed esara closed 5 months ago

esara commented 8 months ago

When connecting to a remote redis cache/database, the server.address and server.port can indicate which host/port the client is connecting to as per https://opentelemetry.io/docs/specs/semconv/database/database-spans/#connection-level-attributes

Expected Behavior

Expected to see server address and port attributes in the redisotel generated traces, for example semconv.ServerAddress and semconv.ServerPort

Current Behavior

These attributes are missing, only the DBConnectionString is populated. But it is unnecessary to parse out the server address from the connection string in the traces.

Possible Solution

Prepopulate server.address and port during instrumentation

Steps to Reproduce

For example, simply adding instrumentation to redis https://github.com/Causely/chaosmania/blob/main/pkg/actions/redis.go#L29 It adds only the connection string

func InstrumentTracing(rdb redis.UniversalClient, opts ...TracingOption) error {
    switch rdb := rdb.(type) {
    case *redis.Client:
        opt := rdb.Options()
        connString := formatDBConnString(opt.Network, opt.Addr)
        rdb.AddHook(newTracingHook(connString, opts...))
        return nil

Context (Environment)

I would like to be able to understand service dependencies including application/redis client to redis server for example in a service graph using the server.address and port attributes.

Possible Implementation

func InstrumentTracing(rdb redis.UniversalClient, opts ...TracingOption) error {
    switch rdb := rdb.(type) {
    case *redis.Client:
        opt := rdb.Options()
        host, portString, err := net.SplitHostPort(opt.Addr)
        if err == nil {
            opts = append(opts, WithAttributes(
                semconv.ServerAddress(host),
            ))
            // Parse the port string to an integer
            port, err := strconv.Atoi(portString)
            if err == nil {
                opts = append(opts, WithAttributes(
                    semconv.ServerPort(port),
                ))
            }
        }
        connString := formatDBConnString(opt.Network, opt.Addr)
        rdb.AddHook(newTracingHook(connString, opts...))
        return nil