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

[integrations] Add udev data path arg to node_exporter #4481

Merged
merged 4 commits into from
Jul 20, 2023

Conversation

sduranc
Copy link
Contributor

@sduranc sduranc commented Jul 17, 2023

PR Description

Node exporter has added udev data to it's diskstats collector. It's possible to set up this path in node_exporter but right now we can't do the same in the grafana agent, so this PR just makes that available. Also, it appears that when node_exporter can't access the path it fails to create disk metrics. This has already been fixed in prometheus operator and in the helm chart.

Which issue(s) this PR fixes

It seems like it fixes #4207
Fixes #4517

Notes to the Reviewer

As far as I understand, no tests need to updated for this change. Please correct me if I'm wong.

PR Checklist

  • CHANGELOG updated
  • Documentation added
  • Tests updated

@sduranc sduranc requested review from a team and clayton-cornell as code owners July 17, 2023 16:24
@CLAassistant
Copy link

CLAassistant commented Jul 17, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@marctc marctc left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! could you also add this new config option to flow component (prometheus.exporter.unix)?

@sduranc
Copy link
Contributor Author

sduranc commented Jul 17, 2023

Thanks for the contribution! could you also add this new config option to flow component (prometheus.exporter.unix)?

Done. @marctc Let me know if something else is needed or if I've done something wrong (I'm off for the day, can take a look tomorrow).

@sduranc sduranc force-pushed the main branch 2 times, most recently from 17d1711 to 797a5b0 Compare July 18, 2023 13:03
@marctc marctc enabled auto-merge (squash) July 18, 2023 13:13
@sduranc
Copy link
Contributor Author

sduranc commented Jul 18, 2023

@marctc any ideas why the CLA isn't working? (I accepted it yesterday and did again just now, but doesn't seem to be reflected) Nevermind, I've fixed it.

auto-merge was automatically disabled July 18, 2023 13:56

Head branch was pushed to by a user without write access

@sduranc sduranc force-pushed the main branch 3 times, most recently from 9734061 to 66c8007 Compare July 18, 2023 14:03
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Docs look OK to me

@clayton-cornell
Copy link
Contributor

@marctc there's a merge conflict. I haven't tried resolving. Is this something for you to sort out, or?

@marctc
Copy link
Contributor

marctc commented Jul 20, 2023

@sduranc could you fix merge conflicts and merget this PR? thanks!

@sduranc
Copy link
Contributor Author

sduranc commented Jul 20, 2023

@sduranc could you fix merge conflicts and merget this PR? thanks!

@marctc Just saw this - it's been fixed now.

@marctc marctc merged commit 074a69a into grafana:main Jul 20, 2023
6 checks passed
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 22, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
4 participants