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

[receiver/filelog] Open files on Windows with FILE_SHARE_DELETE #32037

Open
BinaryFissionGames opened this issue Mar 29, 2024 · 6 comments
Open
Labels
enhancement New feature or request never stale Issues marked with this label will be never staled and automatically removed receiver/filelog

Comments

@BinaryFissionGames
Copy link
Contributor

Component(s)

receiver/filelog

Is your feature request related to a problem? Please describe.

On Windows, files open by the collector cannot be moved/deleted. It would be nice if the collector opened a file with the FILE_SHARE_DELETE share mode

Describe the solution you'd like

When opening files on Windows, with the CreateFile(W|A) function (docs here), a share mode is passed in that dictates how other processes may access this file.

The documentation calls out 3 values that may be combined, "FILE_SHARE_DELETE", "FILE_SHARE_READ", and "FILE_SHARE_WRITE". In Go's default os.Open implementation, "FILE_SHARE_READ|FILE_SHARE_WRITE" is used as this parameter, but having "FILE_SHARE_DELETE" would be useful, because it would allow other processes to delete or rename the file without having to shut down the collector.

It should be possible to construct a file handle using the CreateFile syscall, and turn it into an os.File by using syscall.CreateFile to create the handle, then wrap it using windows.NewFile (the fd here is not of type syscall.Handle, but the implementation is clear that fd should be a handle)

Describe alternatives you've considered

No response

Additional context

No response

@BinaryFissionGames BinaryFissionGames added enhancement New feature or request needs triage New item requiring triage labels Mar 29, 2024
Copy link
Contributor

Pinging code owners:

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

@djaglowski djaglowski removed the needs triage New item requiring triage label Mar 29, 2024
@djaglowski
Copy link
Member

I really like the idea of doing this, assuming it works. It would allow better support on windows and likely would simplify a few aspects of our codebase as well, for example where we currently manage files differently on windows.

cc: @pjanotti, @ChrsMark, @OverOrion

@pjanotti
Copy link
Contributor

pjanotti commented Mar 29, 2024

This makes sense. We may need to be sure that code reading the file is hardened for read failures, but, I imagine that is already the case. Anyway, when implementing we should have a test deleting a file while still reading it to validate the error handling.

@OverOrion
Copy link
Contributor

I like the idea as well.

In theory this could allow the Collector to support the delete_after_read setting on Windows as well (IIUC this is currently unsupported on Windows).

Copy link
Contributor

github-actions bot commented Jun 3, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Jun 3, 2024
@djaglowski djaglowski removed the Stale label Jun 3, 2024
Copy link
Contributor

github-actions bot commented Aug 5, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Aug 5, 2024
@ChrsMark ChrsMark removed the Stale label Aug 5, 2024
@djaglowski djaglowski added the never stale Issues marked with this label will be never staled and automatically removed label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request never stale Issues marked with this label will be never staled and automatically removed receiver/filelog
Projects
None yet
Development

No branches or pull requests

5 participants