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

netdev: Add ifAlias label #3087

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

tomvil
Copy link
Contributor

@tomvil tomvil commented Aug 19, 2024

This is a very handy label to have. Adding it as a option, which can be enabled when needed, to avoid breaking working stuff.

@tomvil tomvil force-pushed the netdev_common_add_ifAlias_label branch from d4241aa to a193625 Compare August 19, 2024 20:03
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

This is a useful idea, but the implementation needs work.

First, any parsing of /sys needs to happen in https://github.com/prometheus/procfs.

Second, this needs to go in netdev_linux.go, not the common file. This would break on non-Linux builds.

@tomvil
Copy link
Contributor Author

tomvil commented Aug 19, 2024

@SuperQ thank you for your response, will fix that. As for the second part, do you want to create some common functions to construct labels and etc (something like getNetDevStats) or do you have something different in mind?

@tomvil
Copy link
Contributor Author

tomvil commented Aug 23, 2024

@SuperQ I've updated the code. Please review once you're available.

@tomvil tomvil changed the title netdev_common: Add ifAlias label netdev: Add ifAlias label Aug 23, 2024
@tomvil tomvil requested a review from SuperQ August 26, 2024 13:01
collector/netdev_common.go Outdated Show resolved Hide resolved
@tomvil tomvil force-pushed the netdev_common_add_ifAlias_label branch 2 times, most recently from 7aff8e8 to 39fd82b Compare August 27, 2024 15:24
@tomvil tomvil requested a review from SuperQ August 27, 2024 15:29
@tomvil tomvil force-pushed the netdev_common_add_ifAlias_label branch from 39fd82b to b6d513c Compare August 27, 2024 15:45
@tomvil tomvil force-pushed the netdev_common_add_ifAlias_label branch from b6d513c to 40dce8f Compare August 27, 2024 19:55
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@tomvil
Copy link
Contributor Author

tomvil commented Sep 11, 2024

ping @discordianfish 🙏

@SuperQ SuperQ merged commit 041d67d into prometheus:master Sep 11, 2024
7 checks passed
@tomvil tomvil deleted the netdev_common_add_ifAlias_label branch September 11, 2024 08:18
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.

2 participants