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

Refactor collectors #1324

Merged
merged 4 commits into from
Nov 13, 2023
Merged

Conversation

jkroepke
Copy link
Member

@jkroepke jkroepke commented Nov 4, 2023

Replaces #1227

Fixes #1259

This PR restructors the collectors into separate packags.

Packags level variables are moved into collectors struct. This allows external libraries to call windows_exporter multiple times.

The defaults collectors are tested under Windows Server 2022 and are working fine. Test and Bench are green.

@mattdurham Maybe you can have an look here, for Grafana Agent.

@breed808 I do some test on windows server and windows server, both working fine. However maybe a RC version as pre-release should be usefull.

@jkroepke jkroepke requested a review from a team as a code owner November 4, 2023 20:47
@jkroepke jkroepke force-pushed the remove-globals branch 13 times, most recently from e5ed61a to 24b7092 Compare November 5, 2023 13:57
@jkroepke jkroepke closed this Nov 11, 2023
@jkroepke jkroepke reopened this Nov 11, 2023
@jkroepke
Copy link
Member Author

@breed808 please let me know if you need assistance here.

Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

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

Phew, I think this is the largest change I've ever reviewed! Great work as always, I've left a few comments querying some of the changes.

pkg/collector/textfile/textfile_test_test.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
pkg/collector/config.go Show resolved Hide resolved
@jkroepke
Copy link
Member Author

jkroepke commented Nov 12, 2023

Thanks for the review!

Sorry for the size. But i had no idea to minimize the changes.

After merge, I would create an additional PR which embeddeds the wmi library, since there is no commit over years.

I'm also interrest to help as maintainer here, since our company is using this exporter in production.

Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
@jkroepke jkroepke force-pushed the remove-globals branch 2 times, most recently from 36524fc to 506e07c Compare November 12, 2023 13:01
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Re-add DHCP perflib dependency

Fixes prometheus-community#1259

Signed-off-by: Jan-Otto Kröpke <[email protected]>
Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for your efforts with this, I imagine it took longer to implement than it did for me to review :)

@breed808 breed808 merged commit 430fdb0 into prometheus-community:master Nov 13, 2023
5 checks passed
@jkroepke jkroepke deleted the remove-globals branch November 13, 2023 06:57
anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 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.

DHCP collector fails in v0.23.1
2 participants