-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Description
Expected Behavior
I was reviewing the source code of the OpenTelemetry hooks and found two possible bugs related to how the attributes are being handled.
The first one is that every time a new node is detected, then we attach the pool name to the list of attributes
go-redis/extra/redisotel/metrics.go
Line 54 in 21bd40a
| conf.attrs = append(conf.attrs, attribute.String("pool.name", conf.poolName)) |
The second bug happens in the function reportPoolStats
go-redis/extra/redisotel/metrics.go
Lines 86 to 88 in 21bd40a
| labels := conf.attrs | |
| idleAttrs := append(labels, attribute.String("state", "idle")) | |
| usedAttrs := append(labels, attribute.String("state", "used")) |
In this case depending on whether the initial labels slice has enough capacity we could get unexpected results, as the code it's not copying the values into a new slice.
See for example this code in the playground that shows the bug https://go.dev/play/p/XBHgyYioH4s
Or check this code in the same file, which correctly handles the attributes
go-redis/extra/redisotel/metrics.go
Lines 215 to 218 in 21bd40a
| attrs := make([]attribute.KeyValue, 0, len(mh.attrs)+2) | |
| attrs = append(attrs, mh.attrs...) | |
| attrs = append(attrs, attribute.String("type", "command")) | |
| attrs = append(attrs, statusAttr(err)) |