Context manager disables buffering on the client

[gilbsgilbs]

Describe the bug

When using the context manager on a client with buffering enabled, the buffering gets disabled globally as soon as a context manager exits.

To Reproduce

import socket

import datadog

sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
sock.setblocking(False)
sock.bind(("localhost", 0))
host, port = sock.getsockname()

def read_sock():
    data = b""
    while True:
        try:
            data += sock.recv(1024)
        except BlockingIOError:
            break
    print("  ", data)

client = datadog.DogStatsd(
    host=host,
    port=port,

    disable_buffering=False,
    max_buffer_len=10000000,
    flush_interval=10000000,
)

print("increment with buffering")
client.increment("foo")
read_sock()

print("flush")
client.flush()
read_sock()

print("increment with context manager")
with client:
    client.increment("foo")
    read_sock()

print("context manager __exit__")
read_sock()

print("increment again")
client.increment("foo")  # Here, the event is sent synchronously, but it shouldn't
read_sock()  

sock.close()

Outputs:

increment with buffering
   b''
flush
   b'foo:1|c\n'
increment with context manager
   b''
context manager __exit__
   b'foo:1|c\n'
increment again
   b'foo:1|c\n'

Buffer after increment again should be empty.

Expected behavior

The buffering configuration of the client shouldn't be changed by the context manager.

Screenshots

N/A

Environment and Versions (please complete the following information):

• library version: 0.43.0
• python version: 3.10

Additional context

The bug seems to be caused by this: https://github.com/DataDog/datadogpy/blob/ee5ac16744407dcbd7a3640ee7b4456536460065/datadog/dogstatsd/base.py#L522

The close_buffer() function (which is called when the context manager exits) always sets the _send function to _send_to_server independently of the client buffering.

[sgnn7]

@gilbsgilbs I'll take a look to confirm but from your well-written issue description it does seem like there's a bug to be fixed here.

[sgnn7]

@gilbsgilbs Just as a sidenote though: once buffering is enabled, there should be no need to do any buffering management except maybe the occasional flush. In all likelihood once buffering gets turned on by default (in some future version), we will very quickly deprecate all manual buffer operation probably.

[gilbsgilbs]

Thanks for the clarification, I'm in line.

• In my case, buffering can be enabled or disabled by configuration. My app has to expect both cases.
• If I'm not mistaken, the context manager guarantees that a flush occurs on exit, which might sometimes come useful, even when buffering is enabled (rather than try: … finally: client.flush()).

[sgnn7]

@gilbsgilbs Fix landed on master so you should have the fix when the next release rolls out.

[gilbsgilbs]

Thanks @sgnn7. Much appreciated work!

[sgnn7]

@gilbsgilbs v0.44.0 is out on Pypi. Give it a try and let me know if you encounter any issues.