-
Notifications
You must be signed in to change notification settings - Fork 698
Add Valkey Instrumentation #3478
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
base: main
Are you sure you want to change the base?
Conversation
@@ -106,7 +106,7 @@ def check_postgres_connection(): | |||
|
|||
@retryable | |||
def check_redis_connection(): | |||
connection = redis.Redis(host=REDIS_HOST, port=REDIS_PORT) | |||
connection = valkey.Redis(host=REDIS_HOST, port=REDIS_PORT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can test both no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you watching for this PR, sir
But, This is a draft of PR.
Could you waiting for ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xrmx
Thank you for your patience. Would you mind taking a look and reviewing this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR is so large that it is difficult to review. I suggest splitting it into several smaller PRs, as that would make more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! This instrumentation PR is actually a near-one-for-one copy of the existing Redis instrumentation, with only the service-specific bits renamed and a handful of Valkey-specific tweaks. To see how small the diff really is, you can run:
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/src \
instrumentation/opentelemetry-instrumentation-valkey/src
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/tests/__init__.py \
instrumentation/opentelemetry-instrumentation-valkey/tests/__init__.py
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py \
instrumentation/opentelemetry-instrumentation-valkey/tests/test_valkey.py
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/LICENCE \
instrumentation/opentelemetry-instrumentation-valkey/LICENCE
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/pyproject.toml \
instrumentation/opentelemetry-instrumentation-valkey/pyproject.toml
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/README.rst \
instrumentation/opentelemetry-instrumentation-valkey/README.rst
git diff --no-index instrumentation/opentelemetry-instrumentation-redis/test-requirements.txt \
instrumentation/opentelemetry-instrumentation-valkey/test-requirements.txt
git diff --no-index tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py \
tests/opentelemetry-docker-tests/tests/valkey/test_valkey_functional.py
Because the core of this PR is simply “copy + rename + small adjustments,” I think a single PR still keeps reviewable granularity. Please let me know if you’d prefer me to split out any particular piece!
669c7f8
to
73d1eca
Compare
73d1eca
to
24ecc80
Compare
Description
This PR introduces a new Valkey instrumentation plugin for the OpenTelemetry Python Contrib library.
users can now automatically capture and export Valkey trace data alongside existing OpenTelemetry spans. This fills the gap for applications leveraging Valkey’s Client library, enabling end-to-end distributed tracing without manual span management.
Notes:
valkey-search
module is not yet GA, related tests are intentionally omitted in this PR.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
instrumentation/opentelemetry-instrumentation-valkey/tests/
covering:db.statement
,db.valkey.args_length
,db.valkey.database_index
,net.peer.name
)opentelemetry-instrumentation-valkey
, verified spans in Jaeger backendtox -e py38-test-instrumentation-valkey
tox -e py39-test-instrumentation-valkey
tox -e py310-test-instrumentation-valkey
tox -e py311-test-instrumentation-valkey
tox -e py312-test-instrumentation-valkey
tox -e py313-test-instrumentation-valkey
tox -e pypy3-test-instrumentation-valkey
tox -e lint-instrumentation-valkey
Does This PR Require a Core Repo Change?
Checklist:
CHANGELOG.md
entry under “New Features”)