Skip to content

Valkey instrumentation support#8228

Merged
amarziali merged 8 commits intoDataDog:masterfrom
AhmadMasry:master
Jan 20, 2025
Merged

Valkey instrumentation support#8228
amarziali merged 8 commits intoDataDog:masterfrom
AhmadMasry:master

Conversation

@AhmadMasry
Copy link
Copy Markdown
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 Enhancements and improvements inst: others All other instrumentations labels Jan 16, 2025
@amarziali amarziali self-assigned this Jan 16, 2025
Comment thread dd-java-agent/instrumentation/valkey-java/build.gradle Outdated
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
Contributor

@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
Copy Markdown

@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
@AhmadMasry
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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 Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants