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

[receiver/vcenter] No Cluster Attribute on Clustered Resource Pool Resources #32535

Closed
StefanKurek opened this issue Apr 18, 2024 · 6 comments
Closed
Labels
bug Something isn't working receiver/vcenter

Comments

@StefanKurek
Copy link
Contributor

StefanKurek commented Apr 18, 2024

Component(s)

receiver/vcenter

What happened?

Description

Currently even though the Resource Pool resources are returned with enough attributes for uniqueness (because of the vcenter.resource_group.inventory_path), the info inside of vcenter.resource_group.inventory_path is not completely standardized and therefore the Datacenter and Cluster info within this attribute cannot reliably be determined. Nested folder structures within vCenter can make both the Datacenter and Cluster names appear in unpredictable locations. Standalone hosts complicate pulling this info further.

A vcenter.cluster.name attribute that is added to all Resource Pool resources (that belong to a Cluster and not a Standalone Host) would both allow users to see this context as well as keep the attributes consistent between the other resources of this receiver.

(The Datacenter attribute version of this issue is already covered in #32531 )

Steps to Reproduce

Collect against any vCenter environment with a vCenter Cluster (not Standalone Host only).

Expected Result

Returned Resource Pool resources should have a vcenter.cluster.name resource attribute

Actual Result

Returned Resource Pool resources have no vcenter.cluster.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

@StefanKurek StefanKurek added bug Something isn't working needs triage New item requiring triage labels Apr 18, 2024
Copy link
Contributor

Pinging code owners:

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

@djaglowski
Copy link
Member

If I understand correctly, we currently describe resource pools with two attributes vcenter.resource_pool.name and vcenter.resource_pool.inventory_path, but the path is not sufficient for identifying the context of the pool. Additionally, a resource pool may belong to either a cluster or a standalone instance. What you're proposing is to add a vcenter.cluster.name to resource pools which belong to a cluster. Is all that right? If so, do we also need to add an attribute to resource pools which belong to standalone instances?

@djaglowski djaglowski removed the needs triage New item requiring triage label Apr 19, 2024
@StefanKurek
Copy link
Contributor Author

@djaglowski that's about right.

Technically the vcenter_recource_pool.inventory_path "mostly" holds the context of the pool.

The issues is that within the inventory path before you even get to the point where the Cluster's name exists, you have X number of nested Datacenter folders and Y nested Cluster folders between the top level folder and the Cluster.
And then in between the Cluster and the Resource Pool you have Z number of nested Resource Pool/vApps.

So the context is there...I think that's why the vcenter.cluster.name wasn't originally added to the Resource Pool resource even though every other Resource under a Cluster contains it. But without extra info, you can't always actually parse out key info like which Cluster the Resource Pool belongs to from the path. And it's good to know the Cluster, as Resource Pools in a clustered system "belong" to a Cluster.

You bring up a good point about the Resource Pools that exist under a standalone Host. In that case, Resource Pools "belong" to a specific Host and not a Cluster. Adding the "vcenter.host.name" attribute to Resource Pools in this situation would be appropriate in my opinion. This matches VMs under a standalone Host as they currently have the 'vcenter.host.name' attribute but not a 'vcenter.cluster.name' attribute (although to be fair, we currently also give VMs under a Cluster the vcenter.host.name attribute as well even though it could change as the VM is migrated from one Host to another within the Cluster.)

At the same time, things are a little better here, as I don't think you can have nested Datacenter folders or ComputeResource folders (this one goes right above the host) in the inventory path. So you technically "could" parse the Host info for a standalone Host Resource Pool from the inventory path. An extra vcenter.host.name attribute would be a nicety for clarity and consistency (with regards to resources that live exclusively under a Host).

@djaglowski
Copy link
Member

I think it boils down to the notion that although inventory path may be user readable, it isn't a good machine readable identifier. I'm in favor of leaving it in place but also adding the more precise attributes for each case as well.

@StefanKurek
Copy link
Contributor Author

@djaglowski for sure. I definitely think it has value and don't want to remove vcenter.resource_pool.inventory_path.

So as a final clarity statement: are you on board for adding a vcenter.cluster.name to Resource Pools (that exist under a Cluster) and vcenter.host.name for Resource Pools (that exist under a standalone Host)?

@djaglowski
Copy link
Member

Sounds correct to me.

djaglowski pushed a commit that referenced this issue Apr 24, 2024
…e Pools (#32538)

**Description:** <Describe what has changed.>
`vcenter.cluster.name` attribute is added to all Resource Pool resources
that belong to a Cluster. In addition, `vcenter.host.name` attribute is
added to all Resource Pool resources that belong to a standalone Host.

This helps to keep attributes consistent between all resource types.
Also, Cluster information can’t automatically be parsed from the
inventory path attribute, as we don’t have the context of nested
folders. So this allows for the ability to tell what Cluster a Resource
Pool belongs to (if any).

**Link to tracking Issue:** <Issue number if applicable>
#32535 

**Testing:** <Describe what testing was performed and which tests were
added.>
Unit/integration tests updated and tested. Local environment tested.

**Documentation:** <Describe the documentation added.>
N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working receiver/vcenter
Projects
None yet
Development

No branches or pull requests

2 participants