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

Fix keyspace configuration in redis key metricset #12913

Merged
merged 12 commits into from
Jul 22, 2019

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Jul 15, 2019

Fix incoherent behaviour when keyspace is specified in the redis host
URL and not in some of the key patterns. If it was not specified it used
the default 0 instead of using the one in the redis host.

Redis doesn't have a direct way to request the keyspace being currently
used by a client, documentation mentions that is responsibility of the
client to keep track of the keyspace used if it changes it using Select.
This solution stores the db number originally configured and uses it
in the key metricset if no other keyspace is configured.

Fixes #12874

Fix incoherent behaviour when keyspace is specified in the redis host
URL and not in some of the key patterns. If it was not specified it used
the default 0 instead of using the one in the redis host.
@jsoriano jsoriano added bug module review needs_backport PR is waiting to be backported to other branches. [zube]: In Review Team:Integrations Label for the Integrations team v7.3.0 labels Jul 15, 2019
@jsoriano jsoriano self-assigned this Jul 15, 2019
@jsoriano jsoriano marked this pull request as ready for review July 15, 2019 17:55
@jsoriano jsoriano requested a review from a team as a code owner July 15, 2019 17:55
@jsoriano jsoriano added [zube]: In Progress in progress Pull request is currently in progress. and removed [zube]: In Review review labels Jul 15, 2019
@jsoriano
Copy link
Member Author

One test is flaky

@jsoriano jsoriano added [zube]: In Review review and removed [zube]: In Progress in progress Pull request is currently in progress. labels Jul 17, 2019
@jsoriano
Copy link
Member Author

One test is flaky

Fixed, the method I was using to get current keyspace was not reliable, according to documentation client should keep track of the keyspace selected, so changed to this method.

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng 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, just need to rebase 😬

@jsoriano
Copy link
Member Author

jenkins, test this again please

1 similar comment
@jsoriano
Copy link
Member Author

jenkins, test this again please

@jsoriano jsoriano merged commit 03d7870 into elastic:master Jul 22, 2019
@jsoriano jsoriano deleted the redis-key-url-with-db branch July 22, 2019 07:39
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Jul 22, 2019
jsoriano added a commit to jsoriano/beats that referenced this pull request Jul 22, 2019
Fix incoherent behaviour when keyspace is specified in the redis host
URL and not in some of the key patterns. If it was not specified it used
the default 0 instead of using the one configured in the redis host.

(cherry picked from commit 03d7870)
jsoriano added a commit that referenced this pull request Jul 23, 2019
…tricset (#12999)

Fix incoherent behaviour when keyspace is specified in the redis host
URL and not in some of the key patterns. If it was not specified it used
the default 0 instead of using the one configured in the redis host.

(cherry picked from commit 03d7870)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
… key metricset (elastic#12999)

Fix incoherent behaviour when keyspace is specified in the redis host
URL and not in some of the key patterns. If it was not specified it used
the default 0 instead of using the one configured in the redis host.

(cherry picked from commit 7bad0a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redis key metricset doesn't use db number if set in the host url
4 participants