From ee2761582ee2585bee115ccc56834dff249fe137 Mon Sep 17 00:00:00 2001 From: Ahmed Mezghani Date: Tue, 8 Mar 2022 11:14:06 +0100 Subject: [PATCH] Apply review suggestions --- datadog/dogstatsd/base.py | 30 ++++++++++---------------- datadog/dogstatsd/container.py | 12 +++-------- tests/unit/dogstatsd/test_container.py | 4 ++-- tests/unit/dogstatsd/test_statsd.py | 28 ++++++++++++------------ 4 files changed, 30 insertions(+), 44 deletions(-) diff --git a/datadog/dogstatsd/base.py b/datadog/dogstatsd/base.py index 9df37bb4e..1b26bf625 100644 --- a/datadog/dogstatsd/base.py +++ b/datadog/dogstatsd/base.py @@ -325,12 +325,12 @@ def __init__( self.default_sample_rate = default_sample_rate # Origin detection - self._container_id_field = None + self._container_id = None if not has_entity_id: origin_detection_enabled = self._is_origin_detection_enabled( container_id, origin_detection_enabled, has_entity_id ) - self._set_container_id_field(container_id, origin_detection_enabled) + self._set_container_id(container_id, origin_detection_enabled) # init telemetry version self._client_tags = [ @@ -769,7 +769,7 @@ def _serialize_metric(self, metric, metric_type, value, tags, sample_rate=1): metric_type, ("|@" + text(sample_rate)) if sample_rate != 1 else "", ("|#" + ",".join(normalize_tags(tags))) if tags else "", - self._get_container_id_field(), + ("|c:" + self._container_id if self._container_id else "") ) def _report(self, metric, metric_type, value, tags, sample_rate): @@ -974,7 +974,8 @@ def event( string = "%s|t:%s" % (string, alert_type) if tags: string = "%s|#%s" % (string, ",".join(tags)) - string = "%s%s" % (string, self._get_container_id_field()) + if self._container_id: + string = "%s|c:%s" % (string, self._container_id) if len(string) > 8 * 1024: raise Exception( @@ -1017,7 +1018,8 @@ def service_check( string = u"{0}|#{1}".format(string, ",".join(tags)) if message: string = u"{0}|m:{1}".format(string, message) - string = u"{0}{1}".format(string, self._get_container_id_field()) + if self._container_id: + string = u"{0}|c:{1}".format(string, self._container_id) if self._telemetry: self.service_checks_count += 1 @@ -1049,31 +1051,21 @@ def _is_origin_detection_enabled(self, container_id, origin_detection_enabled, h value = os.environ.get(ORIGIN_DETECTION_ENABLED, "") return value.lower() not in {"no", "false", "0", "n", "off"} - def _set_container_id_field(self, container_id, origin_detection_enabled): + def _set_container_id(self, container_id, origin_detection_enabled): """ Initializes the container ID. It can either be provided by the user or read from cgroups. """ - container_id_prefix = "|c:" if container_id: - self._container_id_field = container_id_prefix + container_id + self._container_id = container_id return if origin_detection_enabled: try: reader = ContainerID() - self._container_id_field = container_id_prefix + reader.get_container_id() + self._container_id = reader.container_id except Exception as e: log.debug("Couldn't get container ID: %s", str(e)) - self._container_id_field = None - - def _get_container_id_field(self): - """ - Returns the container ID field prefixed and ready to be appended to the datagram. - Returns an empty string if the container ID was not set. - """ - if self._container_id_field: - return self._container_id_field - return "" + self._container_id = None statsd = DogStatsd() diff --git a/datadog/dogstatsd/container.py b/datadog/dogstatsd/container.py index 92b771fcd..fe2e71c78 100644 --- a/datadog/dogstatsd/container.py +++ b/datadog/dogstatsd/container.py @@ -33,7 +33,7 @@ class ContainerID(object): CONTAINER_RE = re.compile(r"(?:.+)?({0}|{1}|{2})(?:\.scope)?$".format(UUID_SOURCE, CONTAINER_SOURCE, TASK_SOURCE)) def __init__(self): - self._container_id = self._read_container_id(self.CGROUP_PATH) + self.container_id = self._read_container_id(self.CGROUP_PATH) def _read_container_id(self, fpath): try: @@ -52,12 +52,6 @@ def _read_container_id(self, fpath): except IOError as e: if e.errno != errno.ENOENT: raise NotImplementedError("Unable to open {}.".format(self.CGROUP_PATH)) - except Exception: - raise UnresolvableContainerID("Unable to read the container ID.") + except Exception as e: + raise UnresolvableContainerID("Unable to read the container ID: " + str(e)) return None - - def get_container_id(self): - """ - Returns the container ID if found, None otherwise. - """ - return self._container_id diff --git a/tests/unit/dogstatsd/test_container.py b/tests/unit/dogstatsd/test_container.py index 058162c66..1dfa61042 100644 --- a/tests/unit/dogstatsd/test_container.py +++ b/tests/unit/dogstatsd/test_container.py @@ -125,12 +125,12 @@ def get_mock_open(read_data=None): ), ), ) -def test_get_container_id(file_contents, expected_container_id): +def test_container_id(file_contents, expected_container_id): with get_mock_open(read_data=file_contents) as mock_open: if file_contents is None: mock_open.side_effect = IOError reader = ContainerID() - assert expected_container_id == reader.get_container_id() + assert expected_container_id == reader.container_id mock_open.assert_called_once_with("/proc/self/cgroup", mode="r") diff --git a/tests/unit/dogstatsd/test_statsd.py b/tests/unit/dogstatsd/test_statsd.py index 740a09d81..edd87d7d2 100644 --- a/tests/unit/dogstatsd/test_statsd.py +++ b/tests/unit/dogstatsd/test_statsd.py @@ -1738,19 +1738,19 @@ def test_histogram_does_not_send_none(self): self.assertIsNone(self.recv()) def test_set_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.set("set", 123) self.assert_equal_telemetry("set:123|s|c:fake-container-id\n", self.recv(2)) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_gauge_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.gauge("gauge", 123.4) self.assert_equal_telemetry("gauge:123.4|g|c:fake-container-id\n", self.recv(2)) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_counter_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.increment("page.views") self.statsd.flush() @@ -1771,22 +1771,22 @@ def test_counter_with_container_field(self): self.statsd.flush() self.assert_equal_telemetry("page.views:-12|c|c:fake-container-id\n", self.recv(2)) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_histogram_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.histogram("histo", 123.4) self.assert_equal_telemetry("histo:123.4|h|c:fake-container-id\n", self.recv(2)) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_timing_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.timing("t", 123) self.assert_equal_telemetry("t:123|ms|c:fake-container-id\n", self.recv(2)) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_event_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" self.statsd.event( "Title", "L1\nL2", @@ -1817,10 +1817,10 @@ def test_event_with_container_field(self): bytes_sent=len(event3), ), ) - self.statsd._container_id_field = None + self.statsd._container_id = None def test_service_check_with_container_field(self): - self.statsd._container_id_field = "|c:fake-container-id" + self.statsd._container_id = "fake-container-id" now = int(time.time()) self.statsd.service_check( "my_check.name", @@ -1842,4 +1842,4 @@ def test_service_check_with_container_field(self): bytes_sent=len(check), ), ) - self.statsd._container_id_field = None + self.statsd._container_id = None