search

solarwinds / docker-plugin-papertrail

A Docker plugin for Papertrail to send and read container logs. As a SolarWinds Innovation Project, this adapter is supported in a best-effort fashion.
https://hub.docker.com/r/solarwinds/papertrail-plugin/
Apache License 2.0
12 stars 3 forks source link

Use container-level flag to determine when logs should be consumed #6

Open seubert opened 5 years ago

seubert commented 5 years ago

This fixes a bug described here:

Here you can see newDriver being called https://github.com/solarwinds/docker-plugin-papertrail/blob/master/main.go#L31 which sets a boolean on the newly initialized driver to true. This boolean determines whether or not consumeLog continues to consume logs (seen here https://github.com/solarwinds/docker-plugin-papertrail/blob/master/driver.go#L112). Now that works when you’re just dealing with one container and not stopping or starting anything. However, when you stop a container and StopLogging gets called (https://github.com/solarwinds/docker-plugin-papertrail/blob/master/http.go#L55 and https://github.com/solarwinds/docker-plugin-papertrail/blob/master/driver.go#L94) that boolean gets set to false which stops log consumption. Nothing calls newDriver again to reset that variable to true so logs never start getting consumed again.

Since the driver is being shared amongst the handlers for the lifetime of the plugin, it can't be holding onto file/container-specific logging state, which it is prior to this patch. The ReadLogs function still depends on loopFactor and I haven't determined whether or not that needs to change but likely so. If so, it will be in a separate PR.

trevrosen commented 5 years ago

This seems to solve the essential problem of putting the state in the right place, and is along the lines I envisioned when I read the bug yesterday.