[receiver/vcenter] Integration test emits duplicate resources

[dmitryax]

The integration test emits two identical resource metrics https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/vcenterreceiver/testdata/integration/expected.yaml#L602-L657.

The test client returns two ResourcePool objects with the following inventory paths in https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/vcenterreceiver/scraper.go#L217:

• /DC0/host/DC0_H0/Resources
• /DC0/host/DC0_C0/Resources

The resource attribute vcenter.resource_pool.name is set to the last part of the inventory paths: Resources.

It's unclear if this is just an integration test or if the same is happening in the real world.

If it's not just an integration test, we need to either adjust the resource attributes to be distinct or introduce some deduplication logic

[github-actions]

Pinging code owners:

• receiver/vcenter: @djaglowski @schmikei

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

Pinging code owners for receiver/vcenter: @djaglowski @schmikei. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[schmikei]

It is my understanding that it is very uncommon to have 2 resource pools named the same from an operations standpoint... I can look into if this is actually limited in vCenter though.

The simulator that govmomi is providing is the source of this issue, but I'll try and do some exploration on what the best way to safeguard against this would be if its the receiver's responsibility to add the InventoryPath as an attribute or more internal object references to safeguard against this.

Snippet from simulator: https://github.com/vmware/govmomi/blob/main/simulator/esx/resource_pool.go

[Dylan-M]

For clarity:

You can have 2 resource pools with the same name, so long as each has a different parent. Parents being one of the following: A host that is not in a cluster, a DRS enabled cluster, or another resource pool.

So, you can have:

• Parent 1
  -- Resource Pool Awesome
  -- Resource Pool Amazing
• Parent 2
  -- Resource Pool Awesome
  -- Resource Pool Wicked

In the example above, you have 2 resource pools named "Awesome", one under each parent. This makes them unique enough for vCenter, due to the backing IDs.

In the original post we have different hosts:
/DC0/host/DC0_H0/Resources
/DC0/host/DC0_C0/Resources

[schmikei]

Thanks for chiming in @Dylan-M, seems like we may want to add the inventory path as a resource attribute in order to safeguard against this kind of setup. @dmitryax would you mind assigning me to this and I'll start working on a PR?

[djaglowski]

seems like we may want to add the inventory path as a resource attribute

Will this account for the three possible scenarios mentioned by @Dylan-M?

Parents being one of the following: A host that is not in a cluster, a DRS enabled cluster, or another resource pool.

[Dylan-M]

@djaglowski It should. The example from the OP is with hosts, but the other 2 options should also have an inventory path. In the event they don't, there is also a unique object ID that can be gathered via another API call. @schmikei should be able to find out from the documentation. I can continue to assist him in looking into it as well.

[Dylan-M]

Actually, looking at it again, the sample paths provided from the tests also definitively include the cluster (DC0).

vCenter uses the following tree structure, within which the resource pools can appear in the 3 places I mentioned:

<root>
+-DC0 # Virtual datacenter
   +-datastore # Datastore folder (created by system)
   | +-Datastore1
   +-host # Host folder (created by system)
   | +-Cluster1
   | | +-Host1
   | | | +-VM1
   | | | +-VM2
   | | | +-hadoop1
   | +-Host2 # Dummy cluster created for non-clustered host
   | | +-Host2
   | | | +- ResourcePool1
   | | | | +-VM3
   | | | +- ResourcePool2
   | | | | +- NestedResourcePool1
   | | | | | +-VM4
   +-vm # VM folder (created by system)
   | +-VM1
   | +-VM2
   | +-Folder1
   | | +-hadoop1
   | | +-NestedFolder1
   | | | +-VM3
   | | | +-VM4

[djaglowski]

Ok, sounds like a good solution. Thanks @schmikei, @Dylan-M