Skip to content
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

Valkey instrumentation support #8228

Merged
merged 8 commits into from
Jan 20, 2025
Merged

Valkey instrumentation support #8228

merged 8 commits into from
Jan 20, 2025

Conversation

AhmadMasry
Copy link
Contributor

@AhmadMasry AhmadMasry commented Jan 16, 2025

What Does This Do

Support instrumentation for Valkey key value store.

Motivation

Valkey is an opensource fork from Redis under the Linux foundation, and cloud providers, like AWS, are providing hosted service for Valkey in AWS Elasticache offering.
Note: Most of the work is based on jedis-4.0 integration with refactoring to use valkey-java instead of jedis, and the embedded-redis library was updated to support newer versions and platforms to test against. Unfortunately, I did not find an embedded valkey provider to test against.

@AhmadMasry AhmadMasry marked this pull request as ready for review January 16, 2025 11:51
@AhmadMasry AhmadMasry requested review from a team as code owners January 16, 2025 11:51
@amarziali amarziali added the tag: community Community contribution label Jan 16, 2025
@PerfectSlayer PerfectSlayer added type: enhancement inst: others All other instrumentations labels Jan 16, 2025
@amarziali amarziali self-assigned this Jan 16, 2025
Copy link
Collaborator

@amarziali amarziali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution. The PR looks good. The muzzle directive, as commented, should be changed in order to let the CI run without issues. After this change will be done, I'll be able to approuve it for merging

@AhmadMasry
Copy link
Contributor Author

You are welcome;
The comments are taken into considerations.
And thank you for your help.

@amarziali amarziali added comp: api Tracer public API and removed comp: api Tracer public API labels Jan 17, 2025
Copy link
Collaborator

@amarziali amarziali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for having improved our codebase. If you did not manage to merge because of internal tests not reported I'll take care on Monday

@AhmadMS1988
Copy link

@amarziali appreciate your help in reviewing the PR.
I can't merge it, please feel free to merge it on Monday if appropriate.

@amarziali amarziali merged commit a0e340b into DataDog:master Jan 20, 2025
58 of 76 checks passed
@AhmadMasry
Copy link
Contributor Author

Hi @amarziali
Can I confirm that it was released on 1.46, because the code is there, but it is not mentioned in the release notes.
Thanks

@mcculls mcculls added this to the 1.46.0 milestone Feb 7, 2025
@amarziali amarziali changed the title Valkey intrumentation support Valkey instrumentation support Feb 7, 2025
@amarziali
Copy link
Collaborator

Hi @amarziali Can I confirm that it was released on 1.46, because the code is there, but it is not mentioned in the release notes. Thanks

Hi @AhmadMasry yes you are right. Indeed the PR has been shipped with 1.46.0. It was missing from release note because the milestone was missing on this PR and our automation did not include in the RL. They are now updated including this PR too. Thanks for having notified us about that issue and thanks again for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: others All other instrumentations tag: community Community contribution type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants