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

Added path.logs and made logging to files the default #1437

Merged
merged 3 commits into from Apr 21, 2016

Conversation

Projects
None yet
2 participants
@tsg
Copy link
Collaborator

commented Apr 20, 2016

This adds the -path.logs CLI flag and corresponding config file
setting. By default it's set to {path.home}/logs. The rotating
file writer now writes to this path by default.

For DEB/RPM, the logs path is set to /var/log/{beatname} which
matches the previous default location. However, this is a breaking
change because the previous default was to send the logs to syslog.

Part of #1371.

Added path.logs and made logging to files the default
This adds the `-path.logs` CLI flag and corresponding config file
setting. By default it's set to `{path.home}/logs`. The rotating
file writer now writes to this path by default.

For DEB/RPM, the logs path is set to `/var/log/{beatname}` which
matches the previous default location. However, this is a breaking
change because the previous default was to send the logs to syslog.

Part of #1371.

@tsg tsg referenced this pull request Apr 20, 2016

Closed

Directory layout and data path #1371

14 of 14 tasks complete

tsg added some commits Apr 20, 2016

@tsg tsg added the review label Apr 21, 2016

default:
logp.WTF("Unknown file type: %s", fileType)
return ""
panic(fmt.Sprintf("Unknown file type: %s", fileType))

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 21, 2016

Collaborator

I would prefer to return an error here instead of using panic.

This comment has been minimized.

Copy link
@tsg

tsg Apr 21, 2016

Author Collaborator

That would mean the function would have to return an error, which would complicate usage for little benefit. Note that because FileType is a custom type, the caller won't be able to pass an invalid value here (compile time error).

The only case where the panic would hit is if someone adds a new entry to the "enum"in this file but forgets to add it here. A panic is fine in that case, IMHO, because it's clearly a development bug that should be discovered before committing.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Apr 21, 2016

LGTM. See comment above.

The migration for the log part from 1.x to 5.x will be quite easy. People only have to make a change in their config file. Should we create a docs with Migration from 1.2.x to 5.0.x where we add this kind of stuff?

@ruflin ruflin merged commit f5b1838 into elastic:master Apr 21, 2016

4 checks passed

CLA Commit author is a member of Elasticsearch
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Build finished.
Details

@tsg tsg deleted the tsg:added_logs_path branch Aug 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.