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

Environment variable NO_WINDOWS_SERVICE to force interactive mode on Windows #2272

Merged

Conversation

agoallikmaa
Copy link
Contributor

Description:
Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.

This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.

Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.

@agoallikmaa agoallikmaa requested a review from a team as a code owner December 8, 2020 12:58
@project-bot project-bot bot added this to In progress in Collector Dec 8, 2020
@codecov
Copy link

codecov bot commented Dec 8, 2020

Codecov Report

Merging #2272 (ac53341) into master (65588cc) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2272   +/-   ##
=======================================
  Coverage   92.03%   92.03%           
=======================================
  Files         272      272           
  Lines       15279    15284    +5     
=======================================
+ Hits        14062    14067    +5     
  Misses        837      837           
  Partials      380      380           
Impacted Files Coverage Δ
internal/collector/telemetry/telemetry.go 90.00% <0.00%> (-4.45%) ⬇️
internal/data/traceid.go 92.30% <0.00%> (+7.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65588cc...ac53341. Read the comment docs.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

NO_WINDOWS_SERVICE needs to be documented. If this solves a known problem it can be documented in docs/troubleshooting.md.

return runInteractive(params)
} else {
return runService(params)
}
}

func checkUseInteractiveMode() (bool, error) {
if value, present := os.LookupEnv("NO_WINDOWS_SERVICE"); present && value != "0" {
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment to explain what this does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Collector automation moved this from In progress to Review in progress Dec 8, 2020
@agoallikmaa
Copy link
Contributor Author

NO_WINDOWS_SERVICE needs to be documented. If this solves a known problem it can be documented in docs/troubleshooting.md.

Added a section to docs/troubleshooting.md

Collector automation moved this from Review in progress to Reviewer approved Dec 16, 2020
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@tigrannajaryan tigrannajaryan merged commit 3595b46 into open-telemetry:master Dec 16, 2020
Collector automation moved this from Reviewer approved to Done Dec 16, 2020
tigrannajaryan pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Mar 24, 2021
…Windows (#2819)

This is a copy of the main repo PR: open-telemetry/opentelemetry-collector#2272 that enables running the collector inside Windows Docker containers. 

Main repo PR description:

> Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.
>
> This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.
>
>Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.
mateuszrzeszutek pushed a commit to mateuszrzeszutek/splunk-otel-collector that referenced this pull request Apr 8, 2021
This is a copy of the main repo PR: open-telemetry/opentelemetry-collector#2272 that enables running the collector inside Windows Docker containers.

Main repo PR description:

> Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.
>
> This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.
>
>Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.

We want to use splunk-otel-collector in our splunk-otel-java
smoke-tests, and that includes Windows tests. This change is required
to make Windows containers work.
mateuszrzeszutek pushed a commit to mateuszrzeszutek/splunk-otel-collector that referenced this pull request Apr 8, 2021
This is a copy of the main repo PR: open-telemetry/opentelemetry-collector#2272 that enables running the collector inside Windows Docker containers.

Main repo PR description:

> Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.
>
> This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.
>
>Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.

We want to use splunk-otel-collector in our splunk-otel-java
smoke-tests, and that includes Windows tests. This change is required
to make Windows containers work.
rmfitzpatrick pushed a commit to signalfx/splunk-otel-collector that referenced this pull request Apr 8, 2021
…254)

This is a copy of the main repo PR: open-telemetry/opentelemetry-collector#2272 that enables running the collector inside Windows Docker containers.

Main repo PR description:

> Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.
>
> This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.
>
>Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.

We want to use splunk-otel-collector in our splunk-otel-java
smoke-tests, and that includes Windows tests. This change is required
to make Windows containers work.
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
…Windows (#2819)

This is a copy of the main repo PR: open-telemetry/opentelemetry-collector#2272 that enables running the collector inside Windows Docker containers. 

Main repo PR description:

> Adding a feature - adds a check for NO_WINDOWS_SERVICE environment variable on Windows to allow forcing interactive mode instead of running as a service.
>
> This is required for using the collector in Windows Docker containers, as at least some of the Windows base images do not support services (fails with "The service process could not connect to the service controller"). Environment variable is used instead of automatic detection of Docker as it is uncertain if images that support services are possible and/or desired.
>
>Running collector in Windows Docker containers is required to perform containerized integration tests of agents on Windows.
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…-telemetry#2272)

Before, the WatchForChanges was not called, now we pass the same WatcherFunc down to the ConfigSources.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants