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

Create missing directories in file logging appenders #117666

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

watson
Copy link
Contributor

@watson watson commented Nov 5, 2021

When using either the FileAppender or the RollingFileAppender with a path to a logfile where the directories doesn't exist, they are now created automatically the first time anything is written to the path.

Release note

Improves the file logging capabilities of Kibana, so that missing directories in the configured file path now are created before Kibana attempts to write to the file.

@watson watson added v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Nov 5, 2021
@watson watson self-assigned this Nov 5, 2021
@@ -73,4 +75,8 @@ export class FileAppender implements DisposableAppender {
});
});
}

private ensureDirectory(path: string) {
mkdirSync(dirname(path), { recursive: true });
Copy link
Contributor Author

@watson watson Nov 11, 2021

Choose a reason for hiding this comment

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

I'm not proud of the sync mkdir here, but I'm not sure there's any other way to do this if it happens in the append step? At least this is just a rare thing only when outputStream is undefined, so probably not any issue if understand the flow correctly

@watson
Copy link
Contributor Author

watson commented Nov 11, 2021

@spalger + @tylersmalley: This PR isn't as important now that I commited a logs/.empty file in #116562 - however, it might still be something we want to carry forward - what do you think?

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

I definitely think it's worth the tiny cost of verifying the directory exists before opening the file.

@watson watson marked this pull request as ready for review November 16, 2021 11:54
@watson watson requested a review from a team as a code owner November 16, 2021 11:54
@watson watson added backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes and removed v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Nov 16, 2021
@watson
Copy link
Contributor Author

watson commented Nov 16, 2021

I'm not sure if this requires a release note?

@azasypkin
Copy link
Member

I'm not sure if this requires a release note?

Just my 2c, I believe it certainly deserves a release_note:enhancement at least.

@watson watson added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Nov 16, 2021
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #4659 succeeded 542465549ccb8eb1cbc9e9aebeece480a4416412

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @watson

@elastic elastic deleted a comment from kibanamachine Nov 16, 2021
@watson watson merged commit 72d3728 into elastic:main Nov 16, 2021
@watson watson deleted the create-log-dirs branch November 16, 2021 13:45
@timroes timroes added the Team:Operations Team label for Operations Team label Dec 6, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:enhancement Team:Operations Team label for Operations Team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants