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.

Sign up for GitHub

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

Jump to bottom

[Metricbeat] Move mongodb.status metricset to ECS #10368

Merged
merged 5 commits into from
Jan 29, 2019
Merged

[Metricbeat] Move mongodb.status metricset to ECS #10368

merged 5 commits into from
Jan 29, 2019

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jan 28, 2019 •
edited
Loading

Only field changed for now is service.version.

The metricset was also changed to use the reporter interface.

@ruflin ruflin added in progress Pull request is currently in progress. module review Metricbeat Metricbeat ecs Team:Integrations Label for the Integrations team labels Jan 28, 2019
@ruflin ruflin self-assigned this Jan 28, 2019
@ruflin ruflin requested a review from a team as a code owner January 28, 2019 12:52
@ruflin ruflin mentioned this pull request Jan 28, 2019
Closed
@ruflin
Copy link
Contributor Author

ruflin commented Jan 28, 2019

@webmat For now I didn't touch anything around process.

@ruflin ruflin mentioned this pull request Jan 28, 2019
Closed
sayden
sayden approved these changes Jan 28, 2019
View reviewed changes
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Code looks good. I'm just wondering why it has detected a full change in data.json when it has actually only removed 2 lines (it passed from 209 to 207 lines). Also the timestamp looks unmodified. Is it ok?

@ruflin
Copy link
Contributor Author

ruflin commented Jan 28, 2019

webmat
webmat reviewed Jan 28, 2019
View reviewed changes
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Seems like some copy/pasta lead to the mongodb module trying to grab stuff from haproxy.* in the events. This probably has to change, right? ;-)

I think keeping the .process custom field is superfluous (more on that below), but I don't feel too strongly about it either way.

So apart from the first point, this looks good 👍

metricbeat/module/mongodb/status/_meta/data.json Outdated Show resolved Hide resolved
metricbeat/module/mongodb/status/status_integration_test.go Outdated Show resolved Hide resolved
@ruflin ruflin requested a review from a team as a code owner January 29, 2019 07:53
ruflin added 3 commits January 29, 2019 08:54
@ruflin
[Metricbeat] Move mongodb.status metricset to ECS
da7f60c 
Only field changed for now is service.version.

The metricset was also changed to use the reporter interface.
@ruflin
apply review feedback
67a846b
@ruflin
update migration fields, changelog, fields.yml
c2e3997
@ruflin ruflin force-pushed the mongodb-status-ecs branch from 2fad96c to c2e3997 Compare January 29, 2019 07:54
@ruflin
Copy link
Contributor Author

ruflin commented Jan 29, 2019

Ready for another round of reviews.

@ruflin ruflin removed the in progress Pull request is currently in progress. label Jan 29, 2019
@ruflin ruflin merged commit e9fe7ed into elastic:master Jan 29, 2019
@ruflin ruflin deleted the mongodb-status-ecs branch January 29, 2019 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webmat webmat webmat left review comments

@sayden sayden sayden approved these changes

@jsoriano jsoriano jsoriano approved these changes

Assignees

@ruflin ruflin

Labels
ecs Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ruflin @webmat @jsoriano @sayden