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

Support clustered redis latency metrics #244

Merged

Conversation

kandogu
Copy link
Contributor

@kandogu kandogu commented Dec 9, 2021

No description provided.

@CLAassistant
Copy link

CLAassistant commented Dec 9, 2021

CLA assistant check
All committers have signed the CLA.

@sdelamo
Copy link
Contributor

sdelamo commented Mar 14, 2022

@kandogu can you describe the PR? What issue are you trying to solve? what feature are you adding?

@kandogu
Copy link
Contributor Author

kandogu commented Apr 11, 2022

Latency metrics are not supported in the clustered client of Redis, I was trying to add latency metrics to the clustered client too as the standalone Redis client. @sdelamo

@kandogu
Copy link
Contributor Author

kandogu commented Jul 7, 2022

@sdelamo can you look at it again?

Copy link
Contributor

@guillermocalvo guillermocalvo left a comment

Choose a reason for hiding this comment

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

Looks good overall, except for the lack of tests.

I personally wouldn't introduce an abstract class to inherit this functionality from, because IMO it makes the implementation a bit more complicated than it needs to be. And I don't foresee many classes extending from it; I may be wrong though.

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

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

We need tests.

@guillermocalvo guillermocalvo changed the title Support clustred redis latency metrics Support clustered redis latency metrics Sep 14, 2023
@guillermocalvo guillermocalvo self-assigned this Sep 14, 2023
@guillermocalvo guillermocalvo added the type: improvement A minor improvement to an existing feature label Sep 14, 2023
@sdelamo sdelamo merged commit 9676bd4 into micronaut-projects:master Nov 9, 2023
4 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants