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

Improve symlink handling in Filebeat #1686

Closed
1 of 2 tasks
ruflin opened this issue May 20, 2016 · 17 comments · Fixed by #1767
Closed
1 of 2 tasks

Improve symlink handling in Filebeat #1686

ruflin opened this issue May 20, 2016 · 17 comments · Fixed by #1767
Labels

Comments

@ruflin
Copy link
Member

ruflin commented May 20, 2016

Currently Filebeat treats symlink as normal files. In case a file appears in the glob as symlink and file, the content is read twice. The following changes should be made:

  • By default symlinks should be skipped (BC break)
  • Introduce a configuration option to enable symlinks following Add support for symlinks #2277
    • Make sure symlinks work properly with overwriting symlinks to new file

Is the second option needed?

For reference also see: https://discuss.elastic.co/t/filebeat-fails-to-harvest-if-a-file-and-a-symlink-to-that-file-is-in-the-same-directory/49743/3

@tsg
Copy link
Contributor

tsg commented May 23, 2016

I would try to go without the second one, and see what backlash we get. I think in this case symlinks can provoke a lot of corner cases.

@ruflin
Copy link
Member Author

ruflin commented May 23, 2016

@tsg What do you mean with the second option? Both changes are required.

@tsg
Copy link
Contributor

tsg commented May 23, 2016

I was thinking to simply don't support symlinks. It's removing
functionality, I know, but I think functionality that was unintentionally
introduced, right?

Tudor
Am 23.05.2016 6:08 nachm. schrieb "Nicolas Ruflin" <[email protected]

:

@tsg https://github.com/tsg What do you mean with the second option?
Both changes are required.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1686 (comment)

@ruflin
Copy link
Member Author

ruflin commented May 23, 2016

@tsg Yes. Ok, then we are on the same page. First remove it and potentially reintroduce it in a second step.

@sudhi-vm
Copy link

We are also seeing this issue. We use github.com/golang/glog for logging. glog typically creates the log file with a long name and places a symlink to the log file in the same folder specified in log_dir flag. In this case, filebeat is reading the file twice.

ruflin added a commit to ruflin/beats that referenced this issue May 30, 2016
Previously symlinks were followed. This had the consequence if a symlink and the file itself existed, the file was read twice. Now symlinks are not followed anymore.

This closes elastic#1686
@ruflin
Copy link
Member Author

ruflin commented May 30, 2016

#1767 now removes following symlinks. I suggest to keep it that way and only introduce a config to enable it in case we get feature requests for it.

andrewkroh pushed a commit that referenced this issue Jun 1, 2016
* Stop following symlinks.

Previously symlinks were followed. This had the consequence if a symlink and the file itself existed, the file was read twice. Now symlinks are not followed anymore.

This closes #1686

* Add symlink for windows

* Turn around params

* Remove symlink comment

* Fix for windows symlink
@shamil
Copy link

shamil commented Sep 3, 2016

We use filebeat to collect logs from Docker logs in Kubernetes cluster. Kubernetes provides a handy path with all container logs which are symlinks to /var/lib/docker/containers. Now that filebeat doesn't have that functionality we no longer can collect the container logs from Kubernetes.

We could just use original path instead of the Kubernetes symlinked one, but we rely on the filename and creating fields based the filename. So we won't be able to upgrade to version 5 until we can re-enable symlinks.

@ruflin
Copy link
Member Author

ruflin commented Sep 5, 2016

@shamil Thanks for reporting this and sharing the insights. Can you share some more details on how you use the filename and what the original filename would look like? I'm not too familiar with kubernetes.

@shamil
Copy link

shamil commented Sep 5, 2016

@ruflin, I'm doing something similar to this: https://github.com/ApsOps/filebeat-kubernetes
let me know if you need more information...

@ruflin
Copy link
Member Author

ruflin commented Sep 6, 2016

Thanks for the link. The interesting part for me is the following:

"/var/log/containers/%{DATA:pod_name}_%{DATA:namespace}_%{GREEDYDATA:container_name}-%{DATA:container_id}.log"

As far as I understand this files are symlinks to a file somewhere else. The file name can be used in logstash to add additional data meta data to the event. Some questions:

  • What is the name of the original files?
  • Where is the original file located?
  • Are these files rotated, means is symlink switched to an other file at some point?

We had in the past the discussion to follow symlinks and read the original file to prevent some symlinks edge cases. But that seems not to work in your case as this would send the original file name and not the one you have above.

@shamil
Copy link

shamil commented Sep 6, 2016

The symlinks automatically updated by kubernetes. The original files arw regular docker logs in /var/lib/docker/containers. The symlinks never updated. They persist for thw lifcycle of the container. The original docker container logs are rotated and didn't caused anybissues so far.

@ruflin
Copy link
Member Author

ruflin commented Sep 6, 2016

  • By symlinks are updated, you mean new ones are created for new files (from new containers) or if the original get rotated, they are pointed to the most recent file?
  • What happens to a container log symlink if the container is removed?

Sorry for all the questions. But I'm kind of surprised that it didn't cause and issues so far and want to understand it more in detail. My assumption so far:

  • As symlink has its own inode and filebeat handles files based on inode. I assume if a symlink is deleted for a file which was rotated and a new symlink (same name) for the new file is created, filebeat will keep the old symlink open and finishes reading the rotated file. The new symlink has a new inode, and it reads it as a new file
  • I'm not sure what would happen, if the file was deleted that the symlink was pointing to or moved to a place without access?

Seems like I need to write some tests to see what the actual behaviour is.

@shamil
Copy link

shamil commented Sep 6, 2016

OK, let me explain.

  1. Symlink is unique to target. Same symlink will never point to different file
  2. If container removed symlink removed as well, there won't be dead symlinks. Kubernetes kubelet manages this.
  3. The files rotated using copytruncate strategy, so it will always stay same target, never new file created or replaced.

I guess Kubernetes guys thought about possible problems and made the necessary steps to avoid the issues you mentioned. I think if it possible to have non-default option for enabling symlinks, that would be great!

Here is an example symlink from /var/log/containers

kubernetes-dashboard-2037278273-exjgk_kube-system_POD-41a6c77b2591440b37e393d843b9a0183eea468df3d68316f14e98119372467d.log -> /var/lib/docker/containers/41a6c77b2591440b37e393d843b9a0183eea468df3d68316f14e98119372467d/41a6c77b2591440b37e393d843b9a0183eea468df3d68316f14e98119372467d-json.log

Thanks

@ruflin
Copy link
Member Author

ruflin commented Sep 7, 2016

A quote from the logrotate docs:

Note that there is a very small time slice between copying the file and truncating it, so some logging data might be lost

With filebeat tailing the files and not harvesting the new file (as it is copied and not found) the chance of loosing some log lines is even higher.

My current conclusion is that this seems to be an acceptable trade off for people. It would mean loosen the filebeat guarantee that all log lines are sent for symlinks, but the same actually applies to all copytruncate use cases.

@ruflin
Copy link
Member Author

ruflin commented Sep 7, 2016

@shamil Lets move our discussion to the open issue here so it is more visible: #2277

@ruflin
Copy link
Member Author

ruflin commented Sep 8, 2016

For people interested in this issue, there is now also a PR with a potential implementation: #2478

@graysonfan
Copy link

@shamil @ruflin , Hi, I use kubernets too, this issures will be fixed? in which verison?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants