fix: Remove Thread/event to fix Django regression

[shawkinsmezmo]

Re-opening #106 in order to make sure these are merged properly and separately. These were prematurely merged into a separate user branch, rather than into master.

Due to the nature in which some third party libraries / frameworks (such as Django) will instantiate and close loggers multiple times between application startup / shutdown, revert a small portion of the recent changes to return to the pattern where the flusher can be started / stopped, based on the condition of the buffer and whether or not there is buffered data. This has been tested against the previous data loss issue and is verified to still ensure that 100% of buffered data is synced.

This change can be broken down into the following functional elements:

• Remove the flusher thread creation from class instantiation. Frameworks such as Django may create multiple instances of this class and then try to do odd things with them, which can lead to the handler's close() method hanging while the flush thread spins endlessly, despite the fact that it is a daemon thread.
• Implement the following behavior:
  • While the lock is held by the buffer thread:
    • If the buffer is not empty but does not have enough data to exceed the flush threshold, start the flush timer. This ensures that data doesn't sit around and become stale.
    • If the buffer data exceeds the flush threshold, then stop the flush timer, and directly schedule a flush.
  • While the lock is held by the flusher thread:
    • If the buffer is empty, or if there is data in the buffer that we want to send, stop the flush timer. There is no need for this thread to be running and attempting to flush data if there is nothing in the buffer.

[shawkinsmezmo]

Most of the changes in this class are due to the removal of the setup / teardown of the unit test class instance of the handler. This is no longer necessary since we are removing the creation of the flusher thread from the lifecycle of the class itself. It also is inconvenient since it gets in the way of some of the mock patching that is used in the unit tests.

[dkhokhlov]

Django may create multiple instances of this class and then try to do odd things with them, which can lead to the handler's close() method hanging while the flush thread spins endlessly, despite the fact that it is a daemon thread.

having multiple instances of LogDNAHandler is valid use case. why close() can be hanging? does not look like a "graceful" close().

[shawkinsmezmo]

Django may create multiple instances of this class and then try to do odd things with them, which can lead to the handler's close() method hanging while the flush thread spins endlessly, despite the fact that it is a daemon thread.

having multiple instances of LogDNAHandler is valid use case. why close() can be hanging? does not look like a "graceful" close().

That issue is resolved with this PR. The issue is a little more complicated then just that there are multiple handlers. It's not a static issue, so much as it's a state issue, and there are interesting things happening under the hood when Django instantiates loggers and other objects and then manages the life-cycles of the instances. The reason this was a problem with the previous implementation is because the previous implementation would instantiate a thread upon instantiation of the class, in the constructor, and then would shut this thread down via an event mechanism when the handler's close() method was called.

Because of what Django is doing under the hood, the handlers are instantiated and then provided as dependencies to the relevant code using Django's dependency injection. This causes issues when some of these handlers have threads that are running. Django will "hang" during startup as it is waiting for certain threads to exit, despite the fact that these threads are marked as daemon threads and thus should not cause applications to wait on their state. This may be in issue for Django and libraries like it because of object de/serialization that happens as dependencies are created and then injected as dependencies.

This PR effectively addresses this by not trying the lifecycle of a thread to class instantiation.

[dkhokhlov]

close_flusher & start_flusher are called under lock except this place.
harmless?

[shawkinsmezmo]

It should be, yes, because close is only called by the python logging library when the application closes. Further, closing / cancelling the flusher is a no-op if the flush timer thread has already started.

[dkhokhlov]

lgtm