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

logp.Logger should expose a Close method to close any open files #179

Closed
belimawr opened this issue Jan 17, 2024 · 0 comments · Fixed by #205
Closed

logp.Logger should expose a Close method to close any open files #179

belimawr opened this issue Jan 17, 2024 · 0 comments · Fixed by #205
Assignees
Labels
Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Comments

@belimawr
Copy link
Contributor

Currently the logp.Logger does not expose any API that allows for closing all open files, this causes problems on Windows making removing the directory with the open file to fail.

One simple way to reproduce it is to create a test that configures logp to log to a file in a folder returned by t.TempDir, when the test ends, there will be an error from testing.go that the folder could not be removed.

One option to work around this issue is to use os.MkdirTemp instead of t.TempDir.

Here is an example of a test that would fail on windows due to open files:

if runtime.GOOS == "windows" {
var err error
tempDir1, err = os.MkdirTemp("", t.Name()+"-*")
if err != nil {
t.Fatalf("could not create temporary directory: %s", err)
}
tempDir2, err = os.MkdirTemp("", t.Name()+"-*")
if err != nil {
t.Fatalf("could not create temporary directory: %s", err)
}
} else {
// We have no problems on Linux and Darwin, so we can rely on t.TempDir
// that will remove the files once the tests finishes.
tempDir1 = t.TempDir()
tempDir2 = t.TempDir()
}

The file rotator we instantiate does expose a Close:

rotator, err := file.NewFileRotator(filename,
file.MaxSizeBytes(cfg.Files.MaxSize),
file.MaxBackups(cfg.Files.MaxBackups),
file.Permissions(os.FileMode(cfg.Files.Permissions)),
file.Interval(cfg.Files.Interval),
file.RotateOnStartup(cfg.Files.RotateOnStartup),
file.RedirectStderr(cfg.Files.RedirectStderr),
)
if err != nil {
return nil, fmt.Errorf("failed to create file rotator: %w", err)
}

func (r *Rotator) Close() error {
r.mutex.Lock()
defer r.mutex.Unlock()
return r.closeFile()
}

however we do not keep a reference for it, completely losing access to it's Close method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants