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

[NEW] instance metadata collector #2712

Closed
wants to merge 3 commits into from

Conversation

HappyTobi
Copy link

Hi all,

I added a new collector for fetch machine / compute information from the provides instance meta data service.
The collector is build to easily integrate other ims services to collect information that are not part of the the filesystem, bios etc.

We think the collector makes totally sense, because the information is node related an should be exposed by them (node_exporter)

Please let me know what you think.
Regards

Signed-off-by: id <happytobi@tscoding.de>
Signed-off-by: id <happytobi@tscoding.de>
)

const (
azureComputeURL = "http://169.254.169.254/metadata/instance/compute?api-version=2021-01-01&format=json"
Copy link
Member

Choose a reason for hiding this comment

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

This reaches out over the network, I'm not sure we want collectors that do this inside the exporter.

Maybe this would be more appropriate as a textfile collector tool

Copy link
Author

@HappyTobi HappyTobi Jun 6, 2023

Choose a reason for hiding this comment

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

It,s a local address, that are only reachable at the node/ server / vm itself.

Azure Example:
https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=windows#usage

There are also other ims for other cloud providers that lists information that are only available through the service.

I'm not sure if creating a txt file is so much better. But I'm open to discuss that.

I think to call the service is fine because the default is that the ims exporter doesn't write anything, an if someone enable the collector with for example the "azure" provider it should be fine.

What do you think about @SuperQ is there someone we should add to discuss that?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but the issue is about the behavior outside of cloud providers. It's an unsecured http connection reaching out from the exporter.

The node_exporter is used in a lot of places that are not cloud.

Copy link
Author

Choose a reason for hiding this comment

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

For sure, the default is that nothing happen.
So no call will be made etc.

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't change the fact that no collector is connecting anywhere today, so this needs more discussion

@kfdm
Copy link

kfdm commented Jun 6, 2023

A bit of a drive by comment on my part, but this seems like something the textfile collector is for 🤔
https://www.robustperception.io/how-to-have-labels-for-machine-roles/

node_ims_info{location="westus",provider="Microsoft.Compute",vm_sku="Standard_A3",vm_zone="1"} 1

Everything in that example seems like something that would be written when a machine is first created 🤔

@HappyTobi
Copy link
Author

Hi @kfdm, @SuperQ, @discordianfish

IMHO we have to think about the "instance metadata service" collector concept globally, not related the the first batch of data that I export for azure, because when the concept / collector will be accepted we will also add more static and dynamic data that is part of the ims.

From my perspective the ims collector should be part of the "node_exporter" because the data is direct related to the node itself, we don't collect data from a 3rd party system like "mysql" etc.

The other thing is, that the concept of "ims" does not only exist on cloud environments, there are also one for "openstack", "esxi" etc."

A textfile collector also works for all the other collectors as well :-D , but I think the important point is, that the data that we are collecting, are direct related to the node and not to a 3rd Party service a db, user services etc.

collector/ims_linux.go Outdated Show resolved Hide resolved
collector/ims_linux.go Outdated Show resolved Hide resolved
@HappyTobi HappyTobi requested a review from SuperQ June 7, 2023 14:49
Signed-off-by: id <happytobi@tscoding.de>
@discordianfish
Copy link
Member

Not completely against it but not convinced we want to support this either. Can you explain which information you can only get via the metadata service that are actually node specific? I don't think locations or provider are something that is node specific but more environment specific. So I'd rather expose e.g locations/AZs from where they are configured

@SuperQ
Copy link
Member

SuperQ commented Jun 15, 2023

I would love to have similar metrics in our environment, but I think this would be better built as a textfile script. I don't see a good reason to hit the metadata API every exporter scrape (For example, typical 2x every 15s). Unless this metdata changes often, once on boot is enough.

@HappyTobi
Copy link
Author

Hi @discordianfish, hi @SuperQ

ok lets first dig into the question about the properties that are only reachable over the ims (specially for azure)

Some of them are dynamic like Loadbalancer information, Scheduled Events

Text vs node_exporter.
Just to tell why I think it make sense to have it at the node_exporter.

  1. Same lifecycle as the others
  2. The concept is not only available at one cloud provider it's a generic thing to export vm information at a http service.
  3. The information that will be provided are vm / compute data and not 3rd party information.
  4. Systemd / cron is required to write the text file that would be read and exported later of the text_node exporter. (Extra work)

Further steps / ideas
We can think of switching from the "hardcoded" properties to something like an "--include" flag to give the possibility to the user to configure the exporter properties that should be returned.
Or an exclude or just expose everything we get from the ims.

I just started with a view properties to discuss / see what you think about the "ims" exporter itself. To extend them etc. would be the next step if the "concept" will be accepted.

Thank you.

@HappyTobi
Copy link
Author

Any updates? @discordianfish @SuperQ

@SuperQ
Copy link
Member

SuperQ commented Jun 30, 2023

Sorry, I don't think we want this collector in the node_exporter. The listed dynamic information is not appropriate for metrics collection.

@HappyTobi
Copy link
Author

@SuperQ

🥲

For us it was required to get some information out of the instance meta data service because there are not available anywhere else.

Some other exporters also export static information about the node, like the "dmi" exporter, thats the reason why I thought It would be possible to integrate the ims service that provides dynamic an static parts.

@frittentheke
Copy link
Contributor

A generic (Python?) script that fetches data from the various metadata services (AWS, Azure, GCP, OpenStack, ...) and then places a metrics file into the textfile collector path would be great.

That could go here: https://github.com/prometheus-community/node-exporter-textfile-collector-scripts/ then and could be extended to support all sorts of providers and their metadata formats ...

@HappyTobi HappyTobi closed this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants