contrib/valkey-io/valkey-go: Add WrapClient to support wrapping an existing client #4543
naoto0822
started this conversation in
Feature Request
Replies: 1 comment
-
|
@rarguelloF I think it makes sense, right? |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Package Name
contrib/valkey-io/valkey-go
Package Version(s)
2.6.0
Describe the feature you'd like
I'd like a WrapClient function to be added that wraps an existing valkey.Client with tracing, similar to how other contrib packages work:
NewClient could then be refactored to call WrapClient internally, keeping full backward compatibility:
Is your feature request related to a problem?
When combining valkeyhook.WithHook with valkeytrace.NewClient, the wrapping order is constrained because NewClient creates the raw client internally. This forces the following structure:
With this order, the ctx received by the hook's Do method does not contain the valkeytrace span. Although valkeytrace calls tracer.StartSpanFromContext and stores the new span in an updated context, that updated context is never propagated back to the hook.
As a result, calling tracer.SpanFromContext(ctx) inside the hook returns the parent span (e.g. the HTTP handler span) instead of the valkeytrace span, causing tags to be set on the wrong span:
With WrapClient, it becomes possible to place the hook inside valkeytrace, so the hook's Do is called with a ctx that already contains the valkeytrace span:
Describe alternatives you've considered
Creating a new span via tracer.StartSpanFromContext inside the hook and attaching tags to it. However, this makes the custom span the parent of the valkeytrace span rather than being associated with it, which results in an unintended trace structure:
Since valkeytrace.client is unexported, it is possible to write a local WrapClient equivalent that duplicates the tracing logic. This works, but introduces maintenance overhead and risks diverging from upstream behavior.
Additional context
Several other contrib/ packages already follow the WrapClient pattern to support use cases like middleware and hook composition (e.g. contrib/go-redis/redis.v8). Adding the same to the valkey package would bring it in line with the rest of the contrib ecosystem.
Beta Was this translation helpful? Give feedback.
All reactions