Skip to content

Commit

Permalink
Apply review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahmed-mez
ahmed-mez committed Mar 8, 2022
1 parent e4ff946 commit ee27615
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 44 deletions.
30 changes: 11 additions & 19 deletions datadog/dogstatsd/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
12 changes: 3 additions & 9 deletions datadog/dogstatsd/container.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
4 changes: 2 additions & 2 deletions tests/unit/dogstatsd/test_container.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
28 changes: 14 additions & 14 deletions tests/unit/dogstatsd/test_statsd.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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

0 comments on commit ee27615

Please sign in to comment.