search

eht16 / python-logstash-async

Python logging handler for sending log events asynchronously to Logstash.
MIT License
182 stars 52 forks source link

Add a wait until all data is sent, before closing socket #81

Closed nazarkhanov closed 1 year ago

nazarkhanov commented 1 year ago

Problem: not all logs reach logstash, when using tcp connection. moreover, all the logs reach the locally working logstash, and when connecting to a remote one, only part of the logs come (in our case)

Cause: the self._sock.close() function call closes the socket before all data from the socket write buffer has time to go over the network.

Adding this fix helped us avoid loss of logs

eht16 commented 1 year ago

Thanks, I could reproduce the described behavior and also that the changes fixed the loss of the send buffer.

I wonder why I never noticed this and I actually never expected this.

While testing, I noticed also reproducable errors on the Logstash side(with and without your changes):

[2023-08-13T16:31:35,401][ERROR][logstash.inputs.tcp      ] somehost/8.8.8.8:52642: closing due:
java.net.SocketException: Connection reset

Those can be fixed when we tell Logstash that we are going to close the socket. I just pushed a further change to call socket.shutdown() which fixed the 'Connection reset' for me.

Would you mind testing this change?

nazarkhanov commented 1 year ago

I couldn't immediately reproduce the error "java.net.SocketException: Connection reset", but by switching to the logstash version on 8.4.1, I was able to reproduce it. On logstash version 7.16.1, only the info message "closing (Connection reset by peer)" was shown. Calling socket.shutdown() really fixed this error on these versions.

Could you release these changes?

eht16 commented 1 year ago

Thanks for the feedback.

Just released the changes in 2.7.0.