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

[vSphere] Add id field to all the metricsets #41097

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[vSphere] Add id field to all the metricsets #41097

wants to merge 10 commits into from
+200 −37

Conversation

niraj-elastic
Copy link
Contributor

Description

Added id field to all the metricsets.

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 added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@niraj-elastic niraj-elastic added enhancement Metricbeat Metricbeat Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team labels Oct 3, 2024
@niraj-elastic niraj-elastic self-assigned this Oct 3, 2024
@niraj-elastic niraj-elastic requested a review from a team as a code owner October 3, 2024 12:28
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 3, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 3, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @niraj-elastic? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 3, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Oct 3, 2024
harnish-elastic
harnish-elastic approved these changes Oct 7, 2024
View reviewed changes
Copy link
Contributor

@harnish-elastic harnish-elastic left a comment

Choose a reason for hiding this comment

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

Minor nits. Otherwise LGTM!

metricbeat/module/vsphere/datastorecluster/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/vsphere/resourcepool/_meta/fields.yml Outdated Show resolved Hide resolved
lucian-ioan
lucian-ioan approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@lucian-ioan lucian-ioan left a comment

Choose a reason for hiding this comment

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

LGTM

kush-elastic
kush-elastic approved these changes Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@kush-elastic kush-elastic left a comment

Choose a reason for hiding this comment

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

LGTM

ishleenk17
ishleenk17 reviewed Oct 18, 2024
View reviewed changes
CHANGELOG.next.asciidoc Outdated
@@ -370,6 +370,7 @@ https:/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Added Cisco Meraki module {pull}40836[40836]
- Added Palo Alto Networks module {pull}40686[40686]
- Restore docker.network.in.* and docker.network.out.* fields in docker module {pull}40968[40968]
- Add `id` field to all the vSphere metricsets. {pill}41097[41097]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Add `id` field to all the vSphere metricsets. {pill}41097[41097]
- Add `id` field to all the vSphere metricsets. {pull}41097[41097]

metricbeat/docs/fields.asciidoc Outdated
*`vsphere.virtualmachine.id`*::
+
--
Virtual machine ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Virtual machine ID.
Unique virtual machine ID.

metricbeat/module/vsphere/host/_meta/fields.yml Outdated
- name: id
type: keyword
description: >
Unique Host ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Unique Host ID.
Unique host ID.

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 18, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b vsphere_add_id_field upstream/vsphere_add_id_field
git merge upstream/main
git push upstream vsphere_add_id_field

ishleenk17
ishleenk17 approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@ishleenk17 ishleenk17 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucian-ioan lucian-ioan lucian-ioan approved these changes

@kush-elastic kush-elastic kush-elastic approved these changes

@ishleenk17 ishleenk17 ishleenk17 approved these changes

@harnish-elastic harnish-elastic harnish-elastic approved these changes

Assignees

@niraj-elastic niraj-elastic

Labels
backport-8.x Automated backport to the 8.x branch with mergify enhancement Metricbeat Metricbeat Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@niraj-elastic @lucian-ioan @kush-elastic @ishleenk17 @harnish-elastic