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

Alternative method to configure NO_WINDOWS_SERVICE #7350

Closed
atoulme opened this issue Mar 9, 2023 · 8 comments · Fixed by #9042
Closed

Alternative method to configure NO_WINDOWS_SERVICE #7350

atoulme opened this issue Mar 9, 2023 · 8 comments · Fixed by #9042
Labels
area:builder os:windows Windows specific issues

Comments

@atoulme
Copy link
Contributor

atoulme commented Mar 9, 2023

Is your feature request related to a problem? Please describe.
The collector uses an environment variable NO_WINDOWS_SERVICE to regulate its behavior on Windows. By default, the collector will run as a windows service. This environment variable disables the behavior.
However, we operate in environments where we cannot set environment variables.

Describe the solution you'd like
We need an alternate way to set this flag, either as command line argument, configuration switch, or other mean.

Describe alternatives you've considered
N/A

Additional context
See signalfx/splunk-otel-collector#2643 for original issue.

@mx-psi
Copy link
Member

mx-psi commented Mar 9, 2023

@atoulme
Copy link
Contributor Author

atoulme commented Mar 9, 2023

Thanks!

I am investigating if we could use godotenv to satisfy this need. Does anyone have reservations with using godotenv?

@bogdandrutu
Copy link
Member

bogdandrutu commented Mar 11, 2023

Alternative would be to have different binaries? That will also simplify our build a bit I think.

@mx-psi
Copy link
Member

mx-psi commented Mar 13, 2023

I like the different binaries option more, more complicated behavior can be built on top of the binaries. This would be a breaking change, so I think we would need a deprecation plan

@atoulme
Copy link
Contributor Author

atoulme commented Mar 13, 2023

How would it simplify the build to have different binaries? Would we have a different binary for Windows only to run the collector as service?

@bogdandrutu
Copy link
Member

Would we have a different binary for Windows only to run the collector as service?

yes, is this common in windows world?

@atoulme
Copy link
Contributor Author

atoulme commented Mar 13, 2023

In the Windows world, it seems to be equally as common to run as a service or interactive. You'll need both.

@cwegener
Copy link

cwegener commented Dec 5, 2023

yes, is this common in windows world?

No.

bogdandrutu pushed a commit that referenced this issue Dec 12, 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 that fails fallback to run as an interactive process. This
follows a recommendation of the Windows [service API
documentation](https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatchera#return-value).
The new code calls `svc.Run` and in case of error checks for
`windows.ERROR_FAILED_SERVICE_CONTROLLER_CONNECT`. If this is the error
the application can safely assume that it is not running as a service.

The duration of a call to `svc.Run` failing with this error was below 3
microseconds in the current GH runner and below 5 microseconds on my
box. 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 tracking Issue:**
Fix #7350

**Testing:**
Fix tested on the Splunk fork that deploys the collector as a service
and as an interactive process on Windows containers.

**Documentation:**
Added changelog.
sokoide pushed a commit to sokoide/opentelemetry-collector that referenced this issue Dec 18, 2023
…-telemetry#9042)

**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 that fails fallback to run as an interactive process. This
follows a recommendation of the Windows [service API
documentation](https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatchera#return-value).
The new code calls `svc.Run` and in case of error checks for
`windows.ERROR_FAILED_SERVICE_CONTROLLER_CONNECT`. If this is the error
the application can safely assume that it is not running as a service.

The duration of a call to `svc.Run` failing with this error was below 3
microseconds in the current GH runner and below 5 microseconds on my
box. 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 tracking Issue:**
Fix open-telemetry#7350

**Testing:**
Fix tested on the Splunk fork that deploys the collector as a service
and as an interactive process on Windows containers.

**Documentation:**
Added changelog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:builder os:windows Windows specific issues
Projects
None yet
4 participants