[receiver/vcenter] No Inventory Path Resource Attribute for VM Resources with vApp Parent

[StefanKurek]

Component(s)

receiver/vcenter

What happened?

Description

Currently, VM resources that are children of a vApp (compared to a Resource Pool) are returned without the vcenter.resource_pool.name and vcenter.resource_pool.inventory_path attributes. While this generally makes sense (even though the vCenter API categorizes VirtualApp as an extension of ResourcePool), this causes these resources to not be very human identifiable.

For these VMs, it would still be equally as useful to see their parent vApp's inventory path and name in resource attributes in order to help identify two same named VMs (and would have better parity with their Resource Pool parented cousins).

Steps to Reproduce

Collect against any vCenter environment with a vApp which contains one or more VMs.

Expected Result

Returned VM resources that are children of a vApp have helpful resource attributes returned that define identifying parent details like inventory path & parent name (similar to VM resources that are children of Resource Pools)

Actual Result

Returned VM resources that are children of a vApp have no helpful resource attributes returned that define identifying parent details like inventory path & parent name

Collector version

v1.5.0/v0.98.0

Environment information

No response

OpenTelemetry Collector configuration

extensions:
  basicauth/prom:
    client_auth:
      username: [PROMUSER]
      password: [PROMPASS]

exporters:
  prometheusremotewrite:
    endpoint: [PROMENDPOINT]
    auth:
      authenticator: basicauth/prom
    resource_to_telemetry_conversion:
      enabled: true # Convert resource attributes to metric labels

processors:
  batch:
    # https://github.com/open-telemetry/opentelemetry-collector/tree/main/processor/batchprocessor

receivers:
  vcenter:
    endpoint: https://[VCENTERHOST]
    username: [VCENTERUSER]
    password: [VCENTERPASS]
    tls:
      insecure: true
    collection_interval: 1m
    initial_delay: 1s

service:
  extensions: [basicauth/prom]
  pipelines:
    metrics:
      receivers: [vcenter]
      processors: [batch]
      exporters: [prometheusremotewrite]

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/vcenter: @djaglowski @schmikei

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

[djaglowski]

This isn't a clear cut bug in my opinion so I think if we added these we need to use a featuregate to roll out the change.

That said, would there be a way to tell that it's not just a normal resource pool? Should we instead add a new attribute vcenter.vapp.name? If we add a new attribute, it can go the slightly easier optional -> default route. If you also want the inventory path, I think a feature gate is appropriate either way.

[StefanKurek]

@djaglowski I actually just noticed that while my description was accurate, I completely messed up the name of this Issue which probably caused some confusion. Fixed now.

To be clear I am only advocating for adding new optional (in the sense that they may or may not be relevant/exist) attributes for the VM resources. These attributes (vcenter.virtual_app.name and vcenter.virtual_app.inventory_path) would be mirrors of the current VM attributes which are also optional in the same sense (vcenter.resource_pool.name and vcenter.resource_pool.path).

Ultimately I'm imagining one set or the other would ever appear on a VM (or none of them for VM templates as is the case today...which is a different thing all together) depending on what type of resource is the direct parent of the VM.

I don't want to touch the Resource Pool resources (we're not currently monitoring metrics for vApps although that may be something we want down the road).

Does this change any of your thoughts about feature gating?

[StefanKurek]

To answer your other question, we we could utilize the existing attributes vcenter.resource_pool.inventory_path and vcenter.resource_pool.name while adding an additional boolean vcenter.resource_pool.is_virtual_app or something like that. I'm not 100% sure if that is where you were going.

This would be accurate from an API's point of view of vCenter resources....but confusing from a user view I think (Resource Pools and vApps and more distinct in the UI vs one being an extension of the other).

[djaglowski]

Thanks for clarifying.

I'm a bit skeptical about adding the inventory path attributes to vms, since this is an attribute that describes the pool/vapp, but isn't really necessary to understand which pool/vapp the vm is within. Basically, I think we should only add that which is necessary to understand the identity of the vm. Does that seem reasonable?

[StefanKurek]

@djaglowski ah, I think I see your concern.

I would say that it "is" necessary to have pool/vapp "inventory path" attributes to determine which pool/vapp the VM is within (also, currently VMs already have BOTH vcenter.resource_pool.inventory_path and vcenter.resource_pool.name...although vcenter.resource_pool.inventory_path includes the name at the end of the path and would have been enough to stand alone on its own).

The reason why it's necessary is because with just vcenter.resource_pool.name, we can't differentiate between two nested sets of pools that have the same Resource Pool name.

So sounds like valid options are:

1. Add both vcenter.virtual_app.inventory_path and vcenter.virtual_app.name attributes as possible options for VMs (will exist for VMs directly under a vApp). This matches the existing functionality of VM resource attributes with regards to Resource Pools and the resource pool attributes.

2. Remove vcenter.resource_pool.name attribute from VM resources. Also add vcenter.virtual_app.inventory_path attribute as a possible option for VMs that exist under vApps.

Thoughts?

[djaglowski]

Ok I guess I missed that inventory path is already being added to vms. Based on that, I support option 1, where we either have the current attributes on vms, or equivalent vapp attributes when applicable.

[crobert-1]

Removing needs triage based on code owner feedback.