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

Add domain state to kvm module #17673

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Add domain state to kvm module #17673

merged 2 commits into from
Apr 28, 2020

Conversation

tomdoherty
Copy link
Contributor

Signed-off-by: Tom Doherty [email protected]

What does this PR do?

This change add's the domain state to the kvm metricbeat module.

Why is it important?

This allows tracking KVM transitions and consolidate resource usage with machine state

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

N/A

How to test this PR locally

I run this using sudo ./metricbeat test modules

Related issues

N/A

Use cases

Allows marrying resource and machine state metrics
Allows monitoring of KVM crashes or events via elastalert

Screenshots

N/A

Logs

N/A

@elasticmachine
Copy link
Collaborator

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?

1 similar comment
@elasticmachine
Copy link
Collaborator

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?

@cla-checker-service
Copy link

cla-checker-service bot commented Apr 10, 2020 •
edited
Loading

💚 CLA has been signed

@tomdoherty
Copy link
Contributor Author

Hi

I hope this is useful for others. This is my first patch for metricbeat so please let me know if there's anything I'm missing

Thanks!

@tomdoherty tomdoherty force-pushed the feature-kvm-status branch 3 times, most recently from d1a6dbf to e1759fa Compare April 11, 2020 11:32
@andresrc andresrc added [zube]: Inbox Team:Services (Deprecated) Label for the former Integrations-Services team labels Apr 13, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@andresrc andresrc added Team:Integrations Label for the Integrations team [zube]: In Review and removed Team:Services (Deprecated) Label for the former Integrations-Services team [zube]: Inbox labels Apr 13, 2020
jsoriano
jsoriano reviewed Apr 27, 2020
View reviewed changes
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks for this change! I have added some small suggestions. You will also need to run mage update from the metricbeat directory and commit the changes, this will be needed to pass tests in CI.

CHANGELOG.asciidoc Outdated Show resolved Hide resolved
metricbeat/module/kvm/status/status.go Outdated Show resolved Hide resolved
metricbeat/module/kvm/status/status.go Outdated Show resolved Hide resolved
metricbeat/module/kvm/status/status.go Outdated Show resolved Hide resolved
@tomdoherty
Copy link
Contributor Author

Hi @jsoriano thanks for the review. I have added your suggestions to the change set and done the mage update too. Would you like me to squash these into one commit and push again?

Thanks again - looking forward to adding more :-)

@jsoriano
Copy link
Member

There is still something failing in CI, you will need to run mage fmt update in x-pack/metricbeat too.

@jsoriano
Copy link
Member

Hey @tomdoherty you were fast 🙂 The last changes in the fields file will probably require another mage update.

@tomdoherty
Copy link
Contributor Author

Thanks @jsoriano! All pushed :-)

@jsoriano jsoriano self-assigned this Apr 27, 2020
@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. v7.8.0 review labels Apr 27, 2020
@jsoriano
Copy link
Member

ok to test

jsoriano
jsoriano approved these changes Apr 27, 2020
View reviewed changes
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Waiting for green.

@jsoriano
Copy link
Member

jenkins, test this please

@jsoriano
Copy link
Member

jenkins run the tests please

jsoriano
jsoriano reviewed Apr 28, 2020
View reviewed changes
metricbeat/module/kvm/status/status.go Outdated Show resolved Hide resolved
metricbeat/module/kvm/status/status.go Outdated Show resolved Hide resolved
@jsoriano jsoriano merged commit ed7fa2a into elastic:master Apr 28, 2020
@jsoriano jsoriano added the test-plan Add this PR to be manual test plan label Apr 28, 2020
@zube zube bot added [zube]: Done test-plan Add this PR to be manual test plan and removed [zube]: In Review test-plan Add this PR to be manual test plan labels Apr 28, 2020
jsoriano pushed a commit to jsoriano/beats that referenced this pull request Apr 28, 2020
@tomdoherty @jsoriano
Add domain state metricset to kvm module (elastic#17673)
b38a78f 
This allows tracking KVM transitions of state and consolidate
resource usage with machine state.

Signed-off-by: Tom Doherty <[email protected]>
(cherry picked from commit ed7fa2a)
@jsoriano jsoriano mentioned this pull request Apr 28, 2020
Merged
6 tasks
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Apr 28, 2020
v1v added a commit to v1v/beats that referenced this pull request Apr 28, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/support-r…
e9ff669 
…unbld

* upstream/master:
  ci: comment PRs with the build status (elastic#17971)
  Add domain state metricset to kvm module (elastic#17673)
  [Agent] Allow CLI paths override (elastic#17781)
  Fix generated metricbeat so create-metricset works. (elastic#18020)
  LIBBEAT: Enhancement replace_string processor for replacing strings values of fields. (elastic#17342)
  Update stale references to _xpack to refer to _license instead (elastic#18030)
  Review dependency patterns collection in Jenkins (elastic#18004)
@andresrc andresrc added test-plan-added This PR has been added to the test plan and removed [zube]: Done labels May 3, 2020
jsoriano added a commit that referenced this pull request May 5, 2020
@jsoriano @tomdoherty
Add domain state metricset to kvm module (#17673) (#18052)
6a2e82e 
This allows tracking KVM transitions of state and consolidate
resource usage with machine state.

Signed-off-by: Tom Doherty <[email protected]>
(cherry picked from commit ed7fa2a)

Co-authored-by: tomdoherty <[email protected]>
This was referenced May 14, 2020
Closed
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoriano jsoriano jsoriano approved these changes

Assignees

@jsoriano jsoriano

Labels
review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tomdoherty @elasticmachine @jsoriano @andresrc