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

fix(#1352): moving the stop signal after svc.Run() #1379

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

DiniFarb
Copy link
Contributor

@DiniFarb DiniFarb commented Jan 8, 2024

As explained in the closed PR #1372 this would be the shorthand fix for #1352.

Just to mention that @jkroepke suggested the it would be better to split the windows service stuff from the exporter itself and create two binaries. Like it is done in the grafana/agent#3243 I also agree with that but this would be a bigger change and take more time to implement which I don't have at the moment.

So in the meantime this PR could fix the stop error and since there is only e little change in when the stop signal is sent, it should not re interduce some old behavior/issues like #1047 or #551.

I have tested this PR with default collectors enabled and windows 11 and windows server 2016 (I don't have more options just yet)

Signed-off-by: Dinifarb <andreas.vogt89@bluewin.ch>
@DiniFarb DiniFarb requested a review from a team as a code owner January 8, 2024 20:35
pkg/initiate/initiate.go Outdated Show resolved Hide resolved
Co-authored-by: Jan-Otto Kröpke <github@jkroepke.de>
Signed-off-by: DiniFarb <andreas.vogt89@bluewin.ch>
@jkroepke
Copy link
Member

jkroepke commented Jan 9, 2024

Thank you!

@jkroepke jkroepke merged commit 5398e91 into prometheus-community:master Jan 9, 2024
5 checks passed
@DiniFarb DiniFarb deleted the win_srv_stop_error_2 branch January 12, 2024 15:37
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.

2 participants