go-redis
go-redis copied to clipboard
redis
fix(redisotel): fix buggy append in reportPoolStats
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/[email protected]#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>}}].
Hello @wzy9607, thank you for the contribution. Can you please add a test for that change with the idleAttrs. Interesting bug and nice catch!
Hello @wzy9607, thank you for the contribution. Can you please add a test for that change with the idleAttrs. Interesting bug and nice catch!
done.
I have solved the merge conflicts, can you take a look again? @monkey92t @ndyakov
@wzy9607 i don't see a response from @monkey92t . Feel free to resolve conflicts and we can merge.
@wzy9607 i don't see a response from @monkey92t . Feel free to resolve conflicts and we can merge.
Conflicts resolved.