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.

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

[vSphere][datastore] Add additional metrics #40441

Merged
merged 28 commits into from
Aug 23, 2024

Conversation

niraj-elastic
Copy link
Contributor

@niraj-elastic niraj-elastic commented Aug 6, 2024

Description

Added following new metrics for the datastore metricset in the vSphere metricbeat module. added new performance API to get more detailed info from the vSphere.

Metrics Type Metrics Field Mappings
Datastore performance  
datastore.read.average (KBps) datastore.read.bytes
datastore.write.average (KBps) datastore.write.bytes
datastore.datastoreIops.average (num) datastore.iops
datastore.totalReadLatency.average (ms) datastore.read.latency.total.ms
datastore.totalWriteLatency.average (ms) datastore.write.latency.total.ms
Datastore Summry  
Datastore.OverallStatus datastore.status
Datastore.Host datastore.host.count
Datastore.Host datastore.host.names
Datastore.Vm datastore.vm.count
Datastore.Vm datastore.vm.names

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.

Related issues

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 6, 2024
Copy link
Contributor

mergify bot commented Aug 6, 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-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@niraj-elastic niraj-elastic marked this pull request as ready for review August 7, 2024 13:08
@niraj-elastic niraj-elastic requested a review from a team as a code owner August 7, 2024 13:08
@niraj-elastic niraj-elastic added enhancement Team:Obs-DC Label for the Data Collection team labels Aug 8, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 8, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/obs-dc (Team:Obs-DC)

@niraj-elastic niraj-elastic added the Metricbeat Metricbeat label Aug 8, 2024
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/data.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/data.go Outdated Show resolved Hide resolved
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@@ -27,7 +27,8 @@ import (
)

func TestFetchEventContents(t *testing.T) {
model := simulator.ESX()
// Creating a new simulator model with VPX server to collect broad range of data.
model := simulator.VPX()
if err := model.Create(); err != nil {
t.Fatal(err)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
defer model.Remove()

nitpick

metricbeat/module/vsphere/datastore/data.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/data_test.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
Copy link
Contributor

mergify bot commented Aug 22, 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_datastore_metricset upstream/vsphere_datastore_metricset
git merge upstream/main
git push upstream vsphere_datastore_metricset

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.

Changes look good, other than one comment that I have left.
Lets merge the conflicts and move ahead.

outputVmNames := make([]string, 0, len(ds.Vm))
for _, ob := range objects {
if ob.Reference().Type == "VirtualMachine" {
name := strings.ReplaceAll(ob.Name, ".", "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, we should not change the names from "." to "_".
But as discussed we can take care of this as part of this issue as this could be a breaking change for existing users and we will have to log that.

…phere_datastore_metricset

Conflicts:
	metricbeat/module/vsphere/fields.go
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore_test.go Outdated Show resolved Hide resolved
@niraj-elastic niraj-elastic requested a review from devamanv August 23, 2024 06:41
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
metricbeat/module/vsphere/datastore/datastore.go Outdated Show resolved Hide resolved
@niraj-elastic niraj-elastic merged commit 94ca509 into elastic:main Aug 23, 2024
29 checks passed
@niraj-elastic niraj-elastic added backport-8.x Automated backport to the 8.x branch with mergify and removed backport-8.x Automated backport to the 8.x branch with mergify labels Sep 10, 2024
@ishleenk17 ishleenk17 added the backport-8.15 Automated backport to the 8.15 branch with mergify label Sep 12, 2024
mergify bot pushed a commit that referenced this pull request Sep 12, 2024
* add data collection

* update changelog

* update datacollection

* update fields.yml

* update test files

* update field discriptions

* address review comments

* address review comments

* address review commetns

* update fields

* update usint test

* address review comments

* update data_test file

* update unit test

* address review comments

* update fields.asci

* address review comments

* update data_test file

* update go.mod file

* update NOTICE.txt

* address review comment

* address review comments

* mage update

* address review comments

* Address review comments

---------

Co-authored-by: Niraj Rathod <[email protected]>
(cherry picked from commit 94ca509)

# Conflicts:
#	metricbeat/module/vsphere/fields.go
ishleenk17 added a commit that referenced this pull request Sep 13, 2024
…40782)

* [vSphere][datastore] Add additional metrics (#40441)

* add data collection

* update changelog

* update datacollection

* update fields.yml

* update test files

* update field discriptions

* address review comments

* address review comments

* address review commetns

* update fields

* update usint test

* address review comments

* update data_test file

* update unit test

* address review comments

* update fields.asci

* address review comments

* update data_test file

* update go.mod file

* update NOTICE.txt

* address review comment

* address review comments

* mage update

* address review comments

* Address review comments

---------

Co-authored-by: Niraj Rathod <[email protected]>
(cherry picked from commit 94ca509)

# Conflicts:
#	metricbeat/module/vsphere/fields.go

* Update fields.go

* update fields.go

---------

Co-authored-by: niraj-elastic <[email protected]>
Co-authored-by: Ishleen Kaur <[email protected]>
Co-authored-by: ishleenk17 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.15 Automated backport to the 8.15 branch with mergify enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants