Issue #7187/fix vsphere metricbeat host #7213

marian-craciunescu · 2018-05-30T15:59:33Z

elasticmachine · 2018-05-30T15:59:34Z

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?

marian-craciunescu · 2018-05-31T08:52:53Z

@ruflin any idea how to fix the build failing , besides reverting the fields.yml ?

ruflin · 2018-05-31T11:40:38Z

@marian-craciunescu Thanks for the PR. You have to run make update in the metricbeat directory so it updates all the generate files. This should get the build green.

ruflin · 2018-05-31T11:42:57Z

metricbeat/module/vsphere/virtualmachine/virtualmachine.go

+			if err != nil {
+				logp.Debug("vsphere", err.Error())
+			} else {
+				event["hostname"] = hostSystem.Summary.Config.Name


Should we put this under the global host.name field? See in ECS: https://github.com/elastic/ecs#host

It will require a bit more changes as with the Fetcher used here a few tricks must be applied to add fields on the root. I can help with that.

I think I can do it..But @bogd raised a valid concern.Maybe somebody is already using this field as it is.I do not know what is the current state for it right now (is vsphere only in beta? )

Yes, vsphere is still in beta to exactly allow such changes. I assume you are thinking of moving it for both metricsets which would make querying easier. Would be great to have that.

Based on the current ECS https://www.elastic.co/guide/en/ecs/current/ecs-host.html, it should be host.id and host.hostname.

Sorry, please ignore my comment above. I just saw #7213 (comment)

karmi · 2018-05-31T12:05:21Z

Hi @marian-craciunescu, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

ruflin

Could you also add a changelog entry? If we add host.name to the other metricset too it should go under breaking changes.

You have to rebase again on master because of a conflicting change in fields.go. No need to try to merge it as it's auto generated with make update.

ruflin · 2018-06-01T06:11:25Z

metricbeat/module/vsphere/virtualmachine/virtualmachine.go

+			if err != nil {
+				logp.Debug("vsphere", err.Error())
+			} else {
+				event["hostname"] = hostSystem.Summary.Config.Name


Yes, vsphere is still in beta to exactly allow such changes. I assume you are thinking of moving it for both metricsets which would make querying easier. Would be great to have that.

ruflin · 2018-06-19T11:01:09Z

@marian-craciunescu Any updates on this one?

marian-craciunescu · 2018-06-21T20:25:05Z

@ruflin I will update the pull request soon.

Eniqmatic · 2018-11-19T12:57:37Z

Any update on this?

alvarolobato · 2018-12-04T12:15:26Z

Closing this PR due to inactivity. @marian-craciunescu please reopen if you decide to work again on this.

marian-craciunescu · 2019-03-15T15:10:56Z

How should I procced to reopen this ?
Also it is not clear for me ...the hostname should be placed in the top field
host.name ? @alvarolobato @ruflin ?

ruflin · 2019-03-18T08:03:02Z

@marian-craciunescu You should see a button below to reopen this PR, if not, let me know and I can reopen.

@webmat WDYT about the placement of host.name in this context?

marian-craciunescu · 2019-03-18T11:09:59Z

@marian-craciunescu You should see a button below to reopen this PR, if not, let me know and I can reopen.
Sorry but I don't have that option.Only Delete branch and Delete fork

ruflin · 2019-03-18T11:30:12Z

@marian-craciunescu Just reopened it, hope that helps.

marian-craciunescu · 2019-03-18T11:32:59Z

@marian-craciunescu Just reopened it, hope that helps.

Thanks..
I will make the checks to pass for now.
Do we still need to put that value in the host.name? If so , I think i need some hints on how to do it in order to be faster.

webmat · 2019-03-18T13:59:14Z

I'd be in favour of leaving the vsphere hostname where it is.

I consider host.* to be populated by add_host_metadata in Beats. Putting source-specific values there may change the semantics of the values seen in the event stream, and may be overwritten by the host metadata processor anyway.

marian-craciunescu · 2019-03-18T14:33:13Z

I'd be in favour of leaving the vsphere hostname where it is.

I consider host.* to be populated by add_host_metadata in Beats. Putting source-specific values there may change the semantics of the values seen in the event stream, and may be overwritten by the host metadata processor anyway.
right now we have
vsphere.virtualmachine.host which is actually a vsphere internal id
[Example: host-167]
vsphere.virtualmachine.name which is something like a tag name put by the user
[ Example CLONE_8june2016 ]

Maybe it is better to have a structure like

{
  "vsphere": {
    "virtualmachine": {
      "host" : {
         "id": "host-167", # old host field
          "name": "CLONE_8june2016", # old name field
          "hostname": "real-host-machine" # newly added field
       }
    }
  }
}

ruflin · 2019-03-19T09:32:43Z

@marian-craciunescu I like what you propose above that you reuse the host object from ECS. One challenge we have here is that this would probably be a breaking change?

marian-craciunescu · 2019-03-19T13:29:57Z

@marian-craciunescu I like what you propose above that you reuse the host object from ECS. One challenge we have here is that this would probably be a breaking change?

I don't think will be a breaking change.Basically 2 fields will not be populated and will not exist anymore .I wil test with an existing mapping on my cluster and I wil get back with more info

…7187/fix-vmsphere-metricbeat-host

…-host

marian-craciunescu · 2019-11-18T17:54:34Z

@marian-craciunescu a new commit pls, adding a line to
https://github.com/elastic/beats/blob/master/CHANGELOG.next.asciidoc

At section ==== Breaking changes then *metricbeat*

Add a line at the bottom of the block like
- vshpere module splits `virtualmachine.host` into `virtualmachine.host.id` and `virtualmachine.host.hostname`. {issue}7187[7187] {pull}7213[7213]

Done I would say

marian-craciunescu · 2019-11-18T19:06:37Z

@odacremolbap the build is failing and it does not seems to be related to what I've did.
This is like an never ending story

odacremolbap

thanks for your work so far, much appreciated.
can you check the 2 nits commented in the review?

odacremolbap · 2019-11-18T19:20:41Z

metricbeat/magefile.go

@@ -51,7 +51,7 @@ import (

 func init() {
 	common.RegisterCheckDeps(update.Update)
-
+	devtools.SetBuildVariableSources(devtools.DefaultBeatBuildVariableSources)


is this needed?
If not, can you remove that line

I will remove it..it should not be be there

odacremolbap · 2019-11-18T19:21:33Z

CHANGELOG.next.asciidoc

@@ -255,7 +255,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix marshaling of ms-since-epoch values in `elasticsearch/cluster_stats` metricset. {pull}14378[14378]
 - Fix checking tagsFilter using length in cloudwatch metricset. {pull}14525[14525]
 - Log bulk failures from bulk API requests to monitoring cluster. {issue}14303[14303] {pull}14356[14356]
-
+- Vshpere module splits `virtualmachine.host` into `virtualmachine.host.id` and `virtualmachine.host.hostname`. {issue}7187[7187] {pull}7213[7213]
 *Packetbeat*


Can you add a blank line between your entry and the packetbeat section?

odacremolbap · 2019-11-18T19:34:20Z

@odacremolbap the build is failing and it does not seems to be related to what I've did.
This is like an never ending story

@marian-craciunescu we are very about to end the never ending story indeed :-)

CI has been a bit flaky lately, lots of new and old systems supported at beats now. We are actively working to improve that, in the meantime we can proceed if a failing test has nothing to do with the changes introduced.

I restarted the failing job and will re-check in a few.

offtopic: if you feel contributing is a pain, we are very willing to hear what we can improve from you. Feel encouraged to tell us if you like.

cc @andresrc

…ps://github.com/marian-craciunescu/beats into issue-elastic#7187/fix-vmsphere-metricbeat-host

marian-craciunescu · 2019-11-18T19:59:36Z

@odacremolbap the build is failing and it does not seems to be related to what I've did.
This is like an never ending story

@marian-craciunescu we are very about to end the never ending story indeed :-)

CI has been a bit flaky lately, lots of new and old systems supported at beats now. We are actively working to improve that, in the meantime we can proceed if the a failing test has nothing to do with the changes introduced.

I restarted the failing job and will re-check in a few.

offtopic: if you feel contributing is a pain, we are very willing to hear what we can improve from you. Feel encouraged to tell us if you like.

cc @andresrc

My problem was actually with the build system you are using.It's overly compliated and sincerely you don't need those auto generated parts (fields.go) .That could be added later , based on a branch trigger.
Except for that I really think there is a communication gap.I know you guys are busy ,but 4-5 days after a comment is posted here and nothing happened it's a little discouraging

odacremolbap · 2019-11-18T22:07:20Z

jenkins test this

odacremolbap · 2019-11-19T09:21:08Z

@marian-craciunescu changes approved, and thanks a lot for the feedback.

can you solve that CHANGELOG.next conflict with master?

…-host

marian-craciunescu · 2019-11-19T11:04:30Z

@marian-craciunescu changes approved, and thanks a lot for the feedback.

can you solve that CHANGELOG.next conflict with master?

Done

* added host.id and host.hostname fields for vsphere (cherry picked from commit 4288a56)

* added host.id and host.hostname fields for vsphere

marian-craciunescu mentioned this pull request May 30, 2018

vSphere Beats module - inconsistent usage of host name vs host ID #7187

Closed

ruflin added module review Metricbeat Metricbeat labels May 31, 2018

ruflin reviewed May 31, 2018

View reviewed changes

marian-craciunescu force-pushed the issue-#7187/fix-vmsphere-metricbeat-host branch from 171b48d to be1fcf9 Compare May 31, 2018 12:15

ruflin reviewed Jun 1, 2018

View reviewed changes

ruflin added the needs update label Jun 20, 2018

ruflin added the Team:Integrations Label for the Integrations team label Nov 21, 2018

alvarolobato closed this Dec 4, 2018

ruflin reopened this Mar 18, 2019

alvarolobato added the [zube]: In Review label Mar 18, 2019

ruflin self-assigned this Jun 5, 2019

marian-craciunescu and others added 2 commits November 18, 2019 19:52

Merge branch 'master' of github.com:elastic/beats into issue-elastic#…

a34ad8d

…7187/fix-vmsphere-metricbeat-host

Merge branch 'master' into issue-elastic#7187/fix-vmsphere-metricbeat…

3b35d29

…-host

odacremolbap suggested changes Nov 18, 2019

View reviewed changes

marian-craciunescu added 2 commits November 18, 2019 21:48

Merge branch 'issue-elastic#7187/fix-vmsphere-metricbeat-host' of htt…

e7bef81

…ps://github.com/marian-craciunescu/beats into issue-elastic#7187/fix-vmsphere-metricbeat-host

rebase with master

f0277ab

odacremolbap self-requested a review November 19, 2019 09:18

odacremolbap approved these changes Nov 19, 2019

View reviewed changes

Merge branch 'master' into issue-elastic#7187/fix-vmsphere-metricbeat…

5c477c6

…-host

odacremolbap added v7.5.0 test-plan Add this PR to be manual test plan and removed needs update test-plan Add this PR to be manual test plan v7.5.0 labels Nov 19, 2019

odacremolbap merged commit 4288a56 into elastic:master Nov 19, 2019

odacremolbap added the v7.5.1 label Nov 19, 2019

zube bot added [zube]: Done and removed [zube]: In Review v7.5.1 labels Nov 19, 2019

odacremolbap mentioned this pull request Nov 19, 2019

Cherry-pick #7213 to 7.x: Issue #7187/fix vsphere metricbeat host #14616

Merged

odacremolbap pushed a commit to odacremolbap/beats that referenced this pull request Nov 19, 2019

Issue elastic#7187/fix vsphere metricbeat host (elastic#7213)

285a987

* added host.id and host.hostname fields for vsphere (cherry picked from commit 4288a56)

odacremolbap added the v7.6.0 label Nov 19, 2019

andresrc removed the [zube]: Done label Nov 19, 2019

odacremolbap pushed a commit that referenced this pull request Nov 20, 2019

Issue #7187/fix vsphere metricbeat host (#7213) (#14616)

b802deb

* added host.id and host.hostname fields for vsphere

Issue #7187/fix vsphere metricbeat host #7213

Issue #7187/fix vsphere metricbeat host #7213

Conversation

marian-craciunescu commented May 30, 2018 • edited Loading

elasticmachine commented May 30, 2018

marian-craciunescu commented May 31, 2018 • edited Loading

ruflin commented May 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karmi commented May 31, 2018

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jun 19, 2018

marian-craciunescu commented Jun 21, 2018

Eniqmatic commented Nov 19, 2018

alvarolobato commented Dec 4, 2018

marian-craciunescu commented Mar 15, 2019

ruflin commented Mar 18, 2019

marian-craciunescu commented Mar 18, 2019

ruflin commented Mar 18, 2019

marian-craciunescu commented Mar 18, 2019

webmat commented Mar 18, 2019

marian-craciunescu commented Mar 18, 2019 • edited Loading

ruflin commented Mar 19, 2019

marian-craciunescu commented Mar 19, 2019

marian-craciunescu commented Nov 18, 2019

marian-craciunescu commented Nov 18, 2019 • edited Loading

odacremolbap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odacremolbap commented Nov 18, 2019 • edited Loading

marian-craciunescu commented Nov 18, 2019

odacremolbap commented Nov 18, 2019

odacremolbap commented Nov 19, 2019

marian-craciunescu commented Nov 19, 2019

marian-craciunescu commented May 30, 2018 •

edited

Loading

marian-craciunescu commented May 31, 2018 •

edited

Loading

marian-craciunescu commented Mar 18, 2019 •

edited

Loading

marian-craciunescu commented Nov 18, 2019 •

edited

Loading

odacremolbap commented Nov 18, 2019 •

edited

Loading