redis / go-redis

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

fix(redisotel): fix buggy append in reportPoolStats #3122

Open wzy9607 opened 2 months ago

wzy9607 commented 2 months ago

The current append twice to conf.attrs approach in reportPoolStats may result in unexpected idleAttrs, due to append can mutate the underlying array of the original slice, as demonstrated at https://go.dev/play/p/jwRMofH91eQ?v=goprev and below.

Also, I replaced metric.WithAttributes in reportPoolStats with metric.WithAttributeSet, since WithAttributes is just WithAttributeSet with some extra works that are not needed here, see https://pkg.go.dev/go.opentelemetry.io/otel/metric@v1.22.0#WithAttributes.

demonstration of the bug

package main

import (
    "fmt"

    "go.opentelemetry.io/otel/attribute"
)

func main() {
    a := make([]attribute.KeyValue, 0, 4)
    a = append(a, attribute.String("a", "a"), attribute.String("b", "b"), attribute.String("c", "c"))
    idle := append(a, attribute.String("state", "idle"))
    used := append(a, attribute.String("state", "used"))
    fmt.Printf("a: %v\n", a)
    fmt.Printf("idle: %v\n", idle)
    fmt.Printf("used: %v\n", used)
}

outputs

a: [{a {4 0 a <nil>}} {b {4 0 b <nil>}} {c {4 0 c <nil>}}]
idle: [{a {4 0 a <nil>}} {b {4 0 b <nil>}} {c {4 0 c <nil>}} {state {4 0 used <nil>}}]
used: [{a {4 0 a <nil>}} {b {4 0 b <nil>}} {c {4 0 c <nil>}} {state {4 0 used <nil>}}]

while the intended result in reportPoolStats usecase is idle: [{a {4 0 a <nil>}} {b {4 0 b <nil>}} {c {4 0 c <nil>}} {state {4 0 idle <nil>}}].