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 run as service detection #4002

Merged
merged 8 commits into from
Dec 5, 2023

Conversation

pjanotti
Copy link
Contributor

@pjanotti pjanotti commented Dec 3, 2023

Description:
Fix run as a service detection on Windows. Instead of trying to detect if it is a service or not, for which both svc.IsAnInteractiveSession() and svc.IsWindowsService() are somehow broken, try to run as a Windows service, if it fails fallback to run as an interactive process. This will add a small cost, which on my machine is under 5 microseconds, to startup in the interactive mode compared to using the NO_WINDOWS_SERVICE environment variable. While this value seems fine for startup I'm keeping the NO_WINDOWS_SERVICE option instead of deprecating it (it doesn't seem worth the trouble of deprecating it).

Link to Splunk idea:
N/A

Testing:
Added benchmark to measure the cost of trying to start as a service (notice that when running as a service there is no extra cost).

Documentation:
Added entry in changelog.

@pjanotti pjanotti requested review from a team as code owners December 3, 2023 21:40
// If environment variable NO_WINDOWS_SERVICE is set with any value other
// than 0, use interactive mode instead of running as a service. This should
// be set in case running as a service is not possible or desired even
// though the current session is not detected to be interactive
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

Fix LGTM, it should be ported to the collector upstream as well.

@pjanotti pjanotti merged commit 41ff5b2 into signalfx:main Dec 5, 2023
131 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants