-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose DialOptions used to create the connection to support instrumentation/tracing features #642
Comments
I don't think exposing |
@stevenh thanks for your response 😄 Actually, the Redis select command doc says:
Do you know if this library supports that? If it does, then it should be trivial to expose the selected db. |
Nope this doesn't currently track the connection DB. I'd even go so far as to say calling |
@stevenh in that case what would be the best approach to support this kind of instrumentation in this library? |
I would write a wrapper to achieve that, I wouldn't add it directly as many people don't need to want it as it would be performance hit, all be it small. |
What every time support opentelemetry? |
Not sure I understand the question, could you clarify? |
This is related to a similar conversation about adding slog support, which has me thinking about a v2 to support easier extension which should consider this use case as well. Would like to peoples thoughts on that as an approach? |
Thinking more about this, if we could enable connection wrapping using dial options that would allow users to enable tracing and logging with zero overhead for others, something to consider. |
Going to close this as a duplicate of #568 we can continue this conversation there. |
Hi 👋
I was wondering if it would be possible to expose the
dialOptions
that were used during any of theDial*
functions to support instrumentation/tracing libraries like OpenTelemetry.As an example, they state Redis instrumentations should at least provide the
db.redis.database_index
field (ref), which is currently inaccessible since this struct and fields are unexported.If this sounds good to you, I'm happy to open a PR implementing this change.
Thanks!
The text was updated successfully, but these errors were encountered: