Skip to content

[Metricbeat] Migrate mysql/galera_status to ReporterV2#11324

Merged
fearful-symmetry merged 2 commits intoelastic:masterfrom
fearful-symmetry:migration/mb/reporterv2/mysql/galera_status
Mar 21, 2019
Merged

[Metricbeat] Migrate mysql/galera_status to ReporterV2#11324
fearful-symmetry merged 2 commits intoelastic:masterfrom
fearful-symmetry:migration/mb/reporterv2/mysql/galera_status

Conversation

@fearful-symmetry
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry commented Mar 19, 2019 •
edited by exekias
Loading

See #10774

This metricset is a bit odd. There's no tests at all, and it looks like there's a lot of copy and paste from the mysql/status metricset. In the future, maybe someone cat put in a PR to clean up the code reuse. The lack of tests is a little weirder. I'm not sure if we need a new docker container added for galera?

@fearful-symmetry fearful-symmetry added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Mar 19, 2019
@fearful-symmetry fearful-symmetry requested a review from a team March 19, 2019 19:27
@ruflin ruflin requested a review from jsoriano March 20, 2019 08:41
@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Mar 20, 2019

The code change overall LGTM. Did you try to run it locally to see if it still works.

I'm sure @jsoriano can add some more details here.

@jsoriano
Copy link
Copy Markdown
Member

jsoriano commented Mar 20, 2019

@fearful-symmetry yes, galera_status is basically a copy of the status metricset and the plan would be to merge one into the other. At the moment this is barely tested and marked as experimental.

Change LGTM, but it'd be good to check in any case that it still works with Galera, try with any docker image available in the docker hub. The Percona one should be maintained (https://hub.docker.com/r/percona/percona-xtradb-cluster/ - XtraDB is the Percona rebranding for Galera).

Btw, I see that we are not printing the experimental warning in New, could you add it here? Something like:

cfgwarn.Experimental("The galera_status metricset is experimental.")

@fearful-symmetry
Copy link
Copy Markdown
Contributor Author

fearful-symmetry commented Mar 20, 2019

@jsoriano Added the log line, and it looks like it runs fine against docker.

@fearful-symmetry fearful-symmetry merged commit 8696d2c into elastic:master Mar 21, 2019
@fearful-symmetry fearful-symmetry deleted the migration/mb/reporterv2/mysql/galera_status branch March 21, 2019 13:10
@sayden sayden mentioned this pull request Mar 21, 2019
Closed
62 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruflin ruflin ruflin approved these changes

@jsoriano jsoriano Awaiting requested review from jsoriano

Assignees

No one assigned

Labels

Metricbeat Metricbeat Team:Integrations Label for the Integrations team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fearful-symmetry @ruflin @jsoriano