[Redis] Rename client input/output buffer metrics #7983
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
@andreycha Thanks for bringing this up. I think on the Metricbeat side it will mean we have to support both variants as Metricbeat could be run against 5 or older versions. |
There was a problem hiding this comment.
@andreycha thanks for addressing this issue! As @ruflin mentioned, we'll have to support both versions, would you like to do it?
We'd have to add the new fields to the fields.yml file, and we probably should fill old and new fields in the final event.
Do you know of other fields that have been renamed?
|
Hi @jsoriano , I sure can add both fields to
If there is a breaking change in Redis, I believe there's no way to avoid it in the beat. Regarding the other fields: I've checked |
|
@andreycha fields can be marked as optional in the schema, you could set the four fields (the old and the new ones). Once the schema is applied we could check if at least one of them of each pair is set, and set the other one for backwards compatibility. |
|
On the other hand, maybe we should wait till redis 5.0 is released in case there are more changes in field names, wdyt? |
|
@jsoriano well, I can rename these ones and you can keep PR unmerged till the release. |
|
@jsoriano I've updated PR, please take a look if I got everything right. |
|
@andreycha after applying the schema, could you fill the 4.x fields if there are 5.x values? this way if someone was using the 4.x fields in a dashboard or something it will continue working with 5.x. |
metricbeat/module/redis/info/data.go
Outdated
| "longest_output_list": c.Int("client_longest_output_list", s.Optional), // Redis 4.x and below | ||
| "biggest_input_buf": c.Int("client_biggest_input_buf", s.Optional), // Redis 4.x and below | ||
| "max_output_buffer": c.Int("client_recent_max_output_buffer", s.Optional), // Redis 5.0 and above | ||
| "max_input_buffer": c.Int("client_recent_max_input_buffer", s.Optional), // Redis 5.0 and above |
There was a problem hiding this comment.
Test build is complaining about format here, could you fix it with gofmt -w metricbeat/module/redis/?
|
@jsoriano alright, that got a bit too far. I'm absolutely not proficient in Go development and have no idea how to fix another build errors and to fill out the old fields. Shall I close this PR and open the issue instead, or it's possible that someone else can finish it? |
|
Hi @andreycha, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
|
@andreycha we are perfectly fine with finishing these changes, you can close this PR and open an issue if you want, as you prefer. If you open a new issue please add a comment here with the link for the record. |
|
I have opened #8167 to continue with this, @andreycha it'd be great if you could review it, thanks! |
|
Hi @jsoriano. sorry, I was on vacation. It's anyway already merged, but I took a look at it and it looks fine. Thank you! |
Hi,
In the upcoming Redis 5.0 release they changed client input/output buffer metrics: redis/redis@be88c0b. This PR is to reflect these changes.
Important: