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.

Sign up for GitHub

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

Jump to bottom

[cmd/opampsupervisor]: Fix supervisor on Windows #34570

Closed
BinaryFissionGames opened this issue Aug 9, 2024 · 3 comments · Fixed by #34573
Closed

[cmd/opampsupervisor]: Fix supervisor on Windows #34570

BinaryFissionGames opened this issue Aug 9, 2024 · 3 comments · Fixed by #34573
Labels
bug Something isn't working cmd/opampsupervisor

Comments

@BinaryFissionGames
Copy link
Contributor

Component(s)

cmd/opampsupervisor

What happened?

Description

Currently, the supervisor doesn't work on windows, nor do our e2e tests work.

The main culprit is the way signals are handled; go does not support sending os.Interrupt on Windows. So instead, we'll have to try and send a graceful shutdown signal on windows a different way.

Collector version

9a04c69

Environment information

Environment

Windows

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response
@BinaryFissionGames BinaryFissionGames added bug Something isn't working needs triage New item requiring triage labels Aug 9, 2024
@BinaryFissionGames
Copy link
Contributor Author

I am currently working on a fix for this and will have a PR out shortly.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@BinaryFissionGames
Copy link
Contributor Author

/label -needs-triage

@github-actions github-actions bot removed the needs triage New item requiring triage label Aug 9, 2024
@BinaryFissionGames BinaryFissionGames mentioned this issue Aug 9, 2024
Merged
@github-actions github-actions bot mentioned this issue Aug 13, 2024
Closed
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this issue Sep 12, 2024
@BinaryFissionGames @djaglowski @f7o
[cmd/opampsupervisor] Fix windows support (open-telemetry#34573)
7e94861 
**Description:** <Describe what has changed.>
* Fixed windows improper signalling for the agent to shutdown.
* Fixed not closing files in e2e tests (windows will error if a file is
not closed when it attempts to delete the temp directory said file is
in)
* Fixed test config templates quoting (windows filepaths would have
trouble since the backslash acts like a character escape)
* Added a workflow to allow running e2e tests for windows

**Link to tracking Issue:** Closes open-telemetry#34570

**Testing:** 
* Existing e2e tests cover this.
* I've manually tested remotely configuring a windows supervisor with
BindPlane.

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working cmd/opampsupervisor
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[cmd/opampsupervisor] Fix windows support observIQ/opentelemetry-collector-contrib
1 participant
@BinaryFissionGames