From ff9f0208e5091a600d02ab331ad15722920c4afb Mon Sep 17 00:00:00 2001 From: Vickenty Fesunov Date: Mon, 4 Dec 2023 18:39:35 +0100 Subject: [PATCH] AMLII-1086 fix stale client_transport tag (#802) initialize reassigns socket_path property on the global statsd instance, but the client_transport tag was computed in the constructor and never updated, causing client to misreport transport as udp when a unix socket was actually used. This patch makes the tag dynamic and updates it every time the socket_path property is updated. --- datadog/dogstatsd/base.py | 17 +++++++++++++---- tests/unit/dogstatsd/test_statsd.py | 12 ++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/datadog/dogstatsd/base.py b/datadog/dogstatsd/base.py index 07aaefdbf..ebb22618e 100644 --- a/datadog/dogstatsd/base.py +++ b/datadog/dogstatsd/base.py @@ -325,14 +325,12 @@ def __init__( self.socket_path = socket_path # type: Optional[text] self.host = None self.port = None - transport = "uds" if not self._max_payload_size: self._max_payload_size = UDS_OPTIMAL_PAYLOAD_LENGTH else: self.socket_path = None self.host = self.resolve_host(host, use_default_route) self.port = int(port) - transport = "udp" if not self._max_payload_size: self._max_payload_size = UDP_OPTIMAL_PAYLOAD_LENGTH @@ -381,7 +379,6 @@ def __init__( self._client_tags = [ "client:py", "client_version:{}".format(__version__), - "client_transport:{}".format(transport), ] self._reset_telemetry() self._telemetry_flush_interval = telemetry_min_flush_interval @@ -417,6 +414,15 @@ def __init__( if not disable_background_sender: self.enable_background_sender(sender_queue_size, sender_queue_timeout) + @property + def socket_path(self): + return self._socket_path + + @socket_path.setter + def socket_path(self, path): + self._socket_path = path + self._transport = "udp" if path is None else "uds" + def enable_background_sender(self, sender_queue_size=0, sender_queue_timeout=0): """ Use a background thread to communicate with the dogstatsd server. @@ -943,7 +949,10 @@ def bytes_dropped(self): return self.bytes_dropped_queue + self.bytes_dropped_writer def _flush_telemetry(self): - telemetry_tags = ",".join(self._add_constant_tags(self._client_tags)) + tags = self._client_tags[:] + tags.append("client_transport:{}".format(self._transport)) + tags.extend(self.constant_tags) + telemetry_tags = ",".join(tags) return TELEMETRY_FORMATTING_STR % ( self.metrics_count, diff --git a/tests/unit/dogstatsd/test_statsd.py b/tests/unit/dogstatsd/test_statsd.py index 2b65ecb7a..8ba0c854a 100644 --- a/tests/unit/dogstatsd/test_statsd.py +++ b/tests/unit/dogstatsd/test_statsd.py @@ -145,9 +145,9 @@ def tearDown(self): """ self._procfs_mock.stop() - def assert_equal_telemetry(self, expected_payload, actual_payload, telemetry=None): + def assert_equal_telemetry(self, expected_payload, actual_payload, telemetry=None, **kwargs): if telemetry is None: - telemetry = telemetry_metrics(bytes_sent=len(expected_payload)) + telemetry = telemetry_metrics(bytes_sent=len(expected_payload), **kwargs) if expected_payload: expected_payload = "\n".join([expected_payload, telemetry]) @@ -705,6 +705,14 @@ def test_udp_socket_ensures_min_receive_buffer(self, mock_socket_create): MIN_SEND_BUFFER_SIZE, ) + def test_socket_path_updates_telemetry(self): + self.statsd.gauge("foo", 1) + self.assert_equal_telemetry("foo:1|g\n", self.recv(2), transport="udp") + self.statsd.socket_path = "/fake/path" + self.statsd._reset_telemetry() + self.statsd.gauge("foo", 2) + self.assert_equal_telemetry("foo:2|g\n", self.recv(2), transport="uds") + def test_distributed(self): """ Measure the distribution of a function's run time using distribution custom metric.