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_cluster] Add support for new metrics in datastore_cluster metricset #40694

Conversation

kush-elastic
Copy link
Collaborator

  • Enhancement

Description

Here are the following metrics to be added for the datastore cluster data stream in the vSphere metricbeat module. Here we added a new performance API to get more detailed information from vSphere.

Metrics Type Metrics API Field Mappings
Datastore cluster childEntity(type= datastore) Summary datastore.names
  childEntity(type= datastore) Summary datastore.count

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

@kush-elastic kush-elastic requested a review from a team as a code owner September 5, 2024 12:55
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 5, 2024
Copy link
Contributor

mergify bot commented Sep 5, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @kush-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

@kush-elastic kush-elastic added enhancement Metricbeat Metricbeat Module:beat The beat Metricbeat module Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team labels Sep 5, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 5, 2024
Copy link
Contributor

mergify bot commented Sep 6, 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 41-collate-and-address-breaking-changes-for-metricsets upstream/41-collate-and-address-breaking-changes-for-metricsets
git merge upstream/main
git push upstream 41-collate-and-address-breaking-changes-for-metricsets

…collate-and-address-breaking-changes-for-metricsets
Comment on lines 68 to 70
func (m *DatastoreClusterMetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are already passing a context, is it necessary to create a new one here?

Is it valuable to control its cancellation here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have planned to remove this from all the metricset but not in current PR.
We will be having common PR for doing all the changes related to all the metricsets.

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.

Small suggestions, otherwise looks good.

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.

Address a nit.
Rest looks good!

@kush-elastic kush-elastic merged commit 83a880f into elastic:main Sep 9, 2024
29 checks passed
@ishleenk17 ishleenk17 added the backport-8.15 Automated backport to the 8.15 branch with mergify label Sep 13, 2024
mergify bot pushed a commit that referenced this pull request Sep 13, 2024
…_cluster metricset (#40694)

* initial commit

* add childEntity

* add changelog entry

* resolve review comments

(cherry picked from commit 83a880f)

# Conflicts:
#	filebeat/input/journald/pkg/journalread/mode_test.go
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/vsphere/datastorecluster/_meta/data.json
#	metricbeat/module/vsphere/datastorecluster/_meta/fields.yml
#	metricbeat/module/vsphere/datastorecluster/data.go
#	metricbeat/module/vsphere/datastorecluster/datastorecluster.go
#	metricbeat/module/vsphere/fields.go
ishleenk17 pushed a commit that referenced this pull request Sep 14, 2024
…_cluster metricset (#40694)

* initial commit

* add childEntity

* add changelog entry

* resolve review comments
ishleenk17 added a commit that referenced this pull request Sep 14, 2024
…_cluster metricset (#40694) (#40831)

* initial commit

* add childEntity

* add changelog entry

* resolve review comments

Co-authored-by: Kush Rana <[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 Module:beat The beat Metricbeat module Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants