From 631c1249567d837190cecb9375b2513f4539c026 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 2 Jan 2019 14:06:01 +0100 Subject: [PATCH 1/7] make joining of server url with API endpoint path a bit smarter fixes #373 --- elasticapm/base.py | 6 ++++-- elasticapm/conf/constants.py | 2 +- tests/client/client_tests.py | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/elasticapm/base.py b/elasticapm/base.py index 7bd1c516d..6bcdf534b 100644 --- a/elasticapm/base.py +++ b/elasticapm/base.py @@ -97,9 +97,11 @@ def __init__(self, config=None, **inline): "max_flush_time": self.config.api_request_time / 1000.0, "max_buffer_size": self.config.api_request_size, } - self._transport = import_string(self.config.transport_class)( - compat.urlparse.urljoin(self.config.server_url, constants.EVENTS_API_PATH), **transport_kwargs + self._api_endpoint_url = compat.urlparse.urljoin( + self.config.server_url if self.config.server_url.endswith("/") else self.config.server_url + "/", + constants.EVENTS_API_PATH, ) + self._transport = import_string(self.config.transport_class)(self._api_endpoint_url, **transport_kwargs) for exc_to_filter in self.config.filter_exception_types or []: exc_to_filter_type = exc_to_filter.split(".")[-1] diff --git a/elasticapm/conf/constants.py b/elasticapm/conf/constants.py index 84882a662..a9038d6d3 100644 --- a/elasticapm/conf/constants.py +++ b/elasticapm/conf/constants.py @@ -1,4 +1,4 @@ -EVENTS_API_PATH = "/intake/v2/events" +EVENTS_API_PATH = "intake/v2/events" TRACE_CONTEXT_VERSION = 0 TRACEPARENT_HEADER_NAME = "elastic-apm-traceparent" diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index 3297a6a60..08f4b6b14 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -804,3 +804,19 @@ def test_ensure_parent_doesnt_change_existing_id(elasticapm_client): span_id = transaction.ensure_parent_id() span_id_2 = transaction.ensure_parent_id() assert span_id == span_id_2 + + +@pytest.mark.parametrize( + "elasticapm_client,expected", + [ + ({"server_url": "http://localhost"}, "http://localhost/intake/v2/events"), + ({"server_url": "http://localhost/"}, "http://localhost/intake/v2/events"), + ({"server_url": "http://localhost:8200"}, "http://localhost:8200/intake/v2/events"), + ({"server_url": "http://localhost:8200/"}, "http://localhost:8200/intake/v2/events"), + ({"server_url": "http://localhost/a"}, "http://localhost/a/intake/v2/events"), + ({"server_url": "http://localhost/a/"}, "http://localhost/a/intake/v2/events"), + ], + indirect=["elasticapm_client"], +) +def test_server_url_joining(elasticapm_client, expected): + assert elasticapm_client._api_endpoint_url == expected From 26467a54baa3005caa37d22ff24e3863e77e32bf Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 2 Jan 2019 14:28:50 +0100 Subject: [PATCH 2/7] add more test cases --- tests/client/client_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index 08f4b6b14..fd02f28e1 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -815,6 +815,8 @@ def test_ensure_parent_doesnt_change_existing_id(elasticapm_client): ({"server_url": "http://localhost:8200/"}, "http://localhost:8200/intake/v2/events"), ({"server_url": "http://localhost/a"}, "http://localhost/a/intake/v2/events"), ({"server_url": "http://localhost/a/"}, "http://localhost/a/intake/v2/events"), + ({"server_url": "http://localhost:8200/a"}, "http://localhost:8200/a/intake/v2/events"), + ({"server_url": "http://localhost:8200/a/"}, "http://localhost:8200/a/intake/v2/events"), ], indirect=["elasticapm_client"], ) From 396bf5cb1eb1826da066131ea4803c784ed05bfb Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 5 Dec 2018 12:02:21 +0100 Subject: [PATCH 3/7] added parsing of cgroups file due to the somewhat underdefined and volatile format of this file, the "parsing" got pretty spaghetti-code-y :( --- elasticapm/utils/docker.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 elasticapm/utils/docker.py diff --git a/elasticapm/utils/docker.py b/elasticapm/utils/docker.py new file mode 100644 index 000000000..18763a523 --- /dev/null +++ b/elasticapm/utils/docker.py @@ -0,0 +1,37 @@ +import os +import re + +DOCKER_ID_PATH = "/proc/self/cgroup" + + +def docker_id(): + if not os.path.exists(DOCKER_ID_PATH): + return + with open(DOCKER_ID_PATH) as f: + return parse_cgroups(f) + + +def parse_cgroups(filehandle): + for line in filehandle: + try: + if "docker" not in line and "kubepods" not in line: + continue + parts = line.split(":") + for part in parts: + if "docker" in part: + docker_id = part.split("/")[-1] + # older docker versions prepend "docker-" and append ".scope" + if "-" in docker_id: + docker_id = re.split("[.-]", docker_id)[1] + return {"container": {"id": docker_id}} + elif "kubepods" in part: + pod_id, docker_id = part.split("/")[-2:] + if "pod" in pod_id: + pod_id = pod_id[pod_id.rindex("pod") + 3 :] + if pod_id.endswith(".slice"): + pod_id = pod_id[:-6] + if docker_id.endswith(".scope"): + docker_id = re.split("[.-]", docker_id)[1] + return {"container": {"id": docker_id}, "pod": {"uid": pod_id}} + except IndexError: + pass From 51fd51733158a15971d5203402f6cc8db5d411f9 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 5 Dec 2018 13:49:00 +0100 Subject: [PATCH 4/7] add docker/kubernetes data to system dict environment variables take precedence over data from /proc/self/cgroup --- elasticapm/base.py | 26 ++++++++++++-- elasticapm/utils/docker.py | 42 ++++++++++++++++------ tests/client/client_tests.py | 69 +++++++++++++++++++++++++++++++++--- tests/utils/docker_tests.py | 37 +++++++++++++++++++ 4 files changed, 157 insertions(+), 17 deletions(-) create mode 100644 tests/utils/docker_tests.py diff --git a/elasticapm/base.py b/elasticapm/base.py index 6bcdf534b..231b828db 100644 --- a/elasticapm/base.py +++ b/elasticapm/base.py @@ -26,7 +26,7 @@ from elasticapm.conf.constants import ERROR from elasticapm.metrics.base_metrics import MetricsRegistry from elasticapm.traces import Tracer, get_transaction -from elasticapm.utils import compat, is_master_process, stacks, varmap +from elasticapm.utils import compat, docker, is_master_process, stacks, varmap from elasticapm.utils.encoding import keyword_field, shorten, transform from elasticapm.utils.module_import import import_string @@ -251,11 +251,33 @@ def get_process_info(self): } def get_system_info(self): - return { + system_data = { "hostname": keyword_field(socket.gethostname()), "architecture": platform.machine(), "platform": platform.system().lower(), } + system_data.update(docker.get_docker_metadata()) + pod_name = os.environ.get("KUBERNETES_POD_NAME") or system_data["hostname"] + changed = False + if "kubernetes" in system_data: + k8s = system_data["kubernetes"] + k8s["pod"]["name"] = pod_name + else: + k8s = {"pod": {"name": pod_name}} + # get kubernetes metadata from environment + if "KUBERNETES_NODE_NAME" in os.environ: + k8s["node"] = {"name": os.environ["KUBERNETES_NODE_NAME"]} + changed = True + if "KUBERNETES_NAMESPACE" in os.environ: + k8s["namespace"] = os.environ["KUBERNETES_NAMESPACE"] + changed = True + if "KUBERNETES_POD_UID" in os.environ: + # this takes precedence over any value from /proc/self/cgroup + k8s["pod"]["uid"] = os.environ["KUBERNETES_POD_UID"] + changed = True + if changed: + system_data["kubernetes"] = k8s + return system_data def _build_metadata(self): return { diff --git a/elasticapm/utils/docker.py b/elasticapm/utils/docker.py index 18763a523..e83eb92ea 100644 --- a/elasticapm/utils/docker.py +++ b/elasticapm/utils/docker.py @@ -4,14 +4,34 @@ DOCKER_ID_PATH = "/proc/self/cgroup" -def docker_id(): +def get_docker_metadata(): + """ + Reads docker/kubernetes metadata (container id, pod id) from /proc/self/cgroup + + The result is a nested dictionary with the detected IDs, e.g. + + { + "container": {"id": "2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63"}, + "pod": {"uid": "90d81341_92de_11e7_8cf2_507b9d4141fa"} + } + + :return: a dictionary with the detected ids or {} + """ if not os.path.exists(DOCKER_ID_PATH): - return + return {} with open(DOCKER_ID_PATH) as f: - return parse_cgroups(f) + return parse_cgroups(f) or {} def parse_cgroups(filehandle): + """ + Reads lines from a file handle and tries to parse docker container IDs and kubernetes Pod IDs. + + See tests.utils.docker_tests.test_cgroup_parsing for a set of test cases + + :param filehandle: + :return: nested dictionary or None + """ for line in filehandle: try: if "docker" not in line and "kubepods" not in line: @@ -19,19 +39,19 @@ def parse_cgroups(filehandle): parts = line.split(":") for part in parts: if "docker" in part: - docker_id = part.split("/")[-1] + container_id = part.split("/")[-1] # older docker versions prepend "docker-" and append ".scope" - if "-" in docker_id: - docker_id = re.split("[.-]", docker_id)[1] - return {"container": {"id": docker_id}} + if "-" in container_id: + container_id = re.split("[.-]", container_id)[1] + return {"container": {"id": container_id}} elif "kubepods" in part: - pod_id, docker_id = part.split("/")[-2:] + pod_id, container_id = part.split("/")[-2:] if "pod" in pod_id: pod_id = pod_id[pod_id.rindex("pod") + 3 :] if pod_id.endswith(".slice"): pod_id = pod_id[:-6] - if docker_id.endswith(".scope"): - docker_id = re.split("[.-]", docker_id)[1] - return {"container": {"id": docker_id}, "pod": {"uid": pod_id}} + if container_id.endswith(".scope"): + container_id = re.split("[.-]", container_id)[1] + return {"container": {"id": container_id}, "pod": {"uid": pod_id}} except IndexError: pass diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index fd02f28e1..4cfcdad85 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -39,15 +39,76 @@ def test_process_info(elasticapm_client): def test_system_info(elasticapm_client): - system_info = elasticapm_client.get_system_info() + # mock docker/kubernetes data here to get consistent behavior if test is run in docker + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: + mocked.return_value = {} + system_info = elasticapm_client.get_system_info() assert {"hostname", "architecture", "platform"} == set(system_info.keys()) +def test_docker_kubernetes_system_info(elasticapm_client): + # mock docker/kubernetes data here to get consistent behavior if test is run in docker + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: + mocked.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + system_info = elasticapm_client.get_system_info() + assert system_info["container"] == {"id": "123"} + assert system_info["kubernetes"] == {"pod": {"uid": "456"}} + + +@mock.patch.dict( + "os.environ", + { + "KUBERNETES_NODE_NAME": "node", + "KUBERNETES_NAMESPACE": "namespace", + "KUBERNETES_POD_NAME": "pod", + "KUBERNETES_POD_UID": "podid", + }, +) +def test_docker_kubernetes_system_info_from_environ(monkeypatch): + # initialize agent only after overriding environment + elasticapm_client = Client() + # mock docker/kubernetes data here to get consistent behavior if test is run in docker + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: + mocked.return_value = {} + system_info = elasticapm_client.get_system_info() + assert "kubernetes" in system_info + assert system_info["kubernetes"] == { + "pod": {"uid": "podid", "name": "pod"}, + "node": {"name": "node"}, + "namespace": "namespace", + } + + +@mock.patch.dict( + "os.environ", + { + "KUBERNETES_NODE_NAME": "node", + "KUBERNETES_NAMESPACE": "namespace", + "KUBERNETES_POD_NAME": "pod", + "KUBERNETES_POD_UID": "podid", + }, +) +def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(monkeypatch): + # initialize agent only after overriding environment + elasticapm_client = Client() + # mock docker/kubernetes data here to get consistent behavior if test is run in docker + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: + mocked.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + system_info = elasticapm_client.get_system_info() + assert "kubernetes" in system_info + assert system_info["kubernetes"] == { + "pod": {"uid": "podid", "name": "pod"}, + "node": {"name": "node"}, + "namespace": "namespace", + } + assert system_info["container"] == {"id": "123"} + + def test_config_by_environment(): - with mock.patch.dict("os.environ", {"ELASTIC_APM_SERVICE_NAME": "app", "ELASTIC_APM_SECRET_TOKEN": "token"}): + with mock.patch.dict("os.environ", {"ELASTIC_APM_SERVICE_NAME": "envapp", "ELASTIC_APM_SECRET_TOKEN": "envtoken"}): client = Client(metrics_interval="0ms") - assert client.config.service_name == "app" - assert client.config.secret_token == "token" + assert client.config.service_name == "envapp" + assert client.config.secret_token == "envtoken" assert client.config.disable_send is False with mock.patch.dict("os.environ", {"ELASTIC_APM_DISABLE_SEND": "true"}): client = Client(metrics_interval="0ms") diff --git a/tests/utils/docker_tests.py b/tests/utils/docker_tests.py new file mode 100644 index 000000000..c37f19880 --- /dev/null +++ b/tests/utils/docker_tests.py @@ -0,0 +1,37 @@ +import mock +import pytest + +from elasticapm.utils import compat, docker + + +@pytest.mark.parametrize( + "test_input,expected", + [ + ( + "12:devices:/docker/051e2ee0bce99116029a13df4a9e943137f19f957f38ac02d6bad96f9b700f76", + {"container": {"id": "051e2ee0bce99116029a13df4a9e943137f19f957f38ac02d6bad96f9b700f76"}}, + ), + ( + "1:name=systemd:/system.slice/docker-cde7c2bab394630a42d73dc610b9c57415dced996106665d427f6d0566594411.scope", + {"container": {"id": "cde7c2bab394630a42d73dc610b9c57415dced996106665d427f6d0566594411"}}, + ), + ( + "1:name=systemd:/kubepods/besteffort/pode9b90526-f47d-11e8-b2a5-080027b9f4fb/15aa6e53-b09a-40c7-8558-c6c31e36c88a", + { + "container": {"id": "15aa6e53-b09a-40c7-8558-c6c31e36c88a"}, + "pod": {"uid": "e9b90526-f47d-11e8-b2a5-080027b9f4fb"}, + }, + ), + ( + "1:name=systemd:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod90d81341_92de_11e7_8cf2_507b9d4141fa.slice/crio-2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63.scope", + { + "container": {"id": "2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63"}, + "pod": {"uid": "90d81341_92de_11e7_8cf2_507b9d4141fa"}, + }, + ), + ], +) +def test_cgroup_parsing(test_input, expected): + f = compat.StringIO(test_input) + result = docker.parse_cgroups(f) + assert expected == result From b0a1b93e95ff9a8a1dc59f8ce9157c8257b27762 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 5 Dec 2018 14:10:53 +0100 Subject: [PATCH 5/7] mock socket.gethostname() to ensure consistent behaviour also, account for larger payload in payload size check, due to added metadata --- tests/client/client_tests.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index 4cfcdad85..76e91f550 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -48,11 +48,14 @@ def test_system_info(elasticapm_client): def test_docker_kubernetes_system_info(elasticapm_client): # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: - mocked.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata, mock.patch( + "socket.gethostname" + ) as mock_gethostname: + mock_metadata.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + mock_gethostname.return_value = "foo" system_info = elasticapm_client.get_system_info() assert system_info["container"] == {"id": "123"} - assert system_info["kubernetes"] == {"pod": {"uid": "456"}} + assert system_info["kubernetes"] == {"pod": {"uid": "456", "name": "foo"}} @mock.patch.dict( @@ -64,12 +67,12 @@ def test_docker_kubernetes_system_info(elasticapm_client): "KUBERNETES_POD_UID": "podid", }, ) -def test_docker_kubernetes_system_info_from_environ(monkeypatch): +def test_docker_kubernetes_system_info_from_environ(): # initialize agent only after overriding environment elasticapm_client = Client() # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: - mocked.return_value = {} + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata: + mock_metadata.return_value = {} system_info = elasticapm_client.get_system_info() assert "kubernetes" in system_info assert system_info["kubernetes"] == { @@ -88,12 +91,15 @@ def test_docker_kubernetes_system_info_from_environ(monkeypatch): "KUBERNETES_POD_UID": "podid", }, ) -def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(monkeypatch): +def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(): # initialize agent only after overriding environment elasticapm_client = Client() # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: - mocked.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata, mock.patch( + "socket.gethostname" + ) as mock_gethostname: + mock_metadata.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} + mock_gethostname.return_value = "foo" system_info = elasticapm_client.get_system_info() assert "kubernetes" in system_info assert system_info["kubernetes"] == { @@ -243,7 +249,7 @@ def test_send(sending_elasticapm_client): for k, v in expected_headers.items(): assert seen_headers[k] == v - assert 250 < request.content_length < 350 + assert 250 < request.content_length < 400 @pytest.mark.parametrize("sending_elasticapm_client", [{"disable_send": True}], indirect=True) From 7585e0a54ebb6a6fd6762fa1b0007ae70ef700a9 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 19 Dec 2018 16:19:13 +0100 Subject: [PATCH 6/7] refactor cgroup parsing to use regexes, heavily inspired by elastic/apm-agent-go#342 (as in, pretty much copied from elastic/apm-agent-go#342) --- elasticapm/base.py | 4 +- elasticapm/utils/cgroup.py | 75 +++++++++++++++++++ elasticapm/utils/docker.py | 57 -------------- tests/client/client_tests.py | 8 +- .../{docker_tests.py => cgroup_tests.py} | 6 +- 5 files changed, 84 insertions(+), 66 deletions(-) create mode 100644 elasticapm/utils/cgroup.py delete mode 100644 elasticapm/utils/docker.py rename tests/utils/{docker_tests.py => cgroup_tests.py} (92%) diff --git a/elasticapm/base.py b/elasticapm/base.py index 231b828db..1333af735 100644 --- a/elasticapm/base.py +++ b/elasticapm/base.py @@ -26,7 +26,7 @@ from elasticapm.conf.constants import ERROR from elasticapm.metrics.base_metrics import MetricsRegistry from elasticapm.traces import Tracer, get_transaction -from elasticapm.utils import compat, docker, is_master_process, stacks, varmap +from elasticapm.utils import cgroup, compat, is_master_process, stacks, varmap from elasticapm.utils.encoding import keyword_field, shorten, transform from elasticapm.utils.module_import import import_string @@ -256,7 +256,7 @@ def get_system_info(self): "architecture": platform.machine(), "platform": platform.system().lower(), } - system_data.update(docker.get_docker_metadata()) + system_data.update(cgroup.get_cgroup_container_metadata()) pod_name = os.environ.get("KUBERNETES_POD_NAME") or system_data["hostname"] changed = False if "kubernetes" in system_data: diff --git a/elasticapm/utils/cgroup.py b/elasticapm/utils/cgroup.py new file mode 100644 index 000000000..3cff02caf --- /dev/null +++ b/elasticapm/utils/cgroup.py @@ -0,0 +1,75 @@ +import os +import re + +CGROUP_PATH = "/proc/self/cgroup" + +SYSTEMD_SCOPE_SUFFIX = ".scope" + +kubepods_regexp = re.compile( + r"(?:^/kubepods/[^/]+/pod([^/]+)$)|(?:^/kubepods\.slice/kubepods-[^/]+\.slice/kubepods-[^/]+-pod([^/]+)\.slice$)" +) + +container_id_regexp = re.compile("^[0-9A-Fa-f]{64}$") + + +def get_cgroup_container_metadata(): + """ + Reads docker/kubernetes metadata (container id, pod id) from /proc/self/cgroup + + The result is a nested dictionary with the detected IDs, e.g. + + { + "container": {"id": "2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63"}, + "pod": {"uid": "90d81341_92de_11e7_8cf2_507b9d4141fa"} + } + + :return: a dictionary with the detected ids or {} + """ + if not os.path.exists(CGROUP_PATH): + return {} + with open(CGROUP_PATH) as f: + return parse_cgroups(f) or {} + + +def parse_cgroups(filehandle): + """ + Reads lines from a file handle and tries to parse docker container IDs and kubernetes Pod IDs. + + See tests.utils.docker_tests.test_cgroup_parsing for a set of test cases + + :param filehandle: + :return: nested dictionary or None + """ + for line in filehandle: + parts = line.strip().split(":") + if len(parts) != 3: + continue + cgroup_path = parts[2] + + # Depending on the filesystem driver used for cgroup + # management, the paths in /proc/pid/cgroup will have + # one of the following formats in a Docker container: + # + # systemd: /system.slice/docker-.scope + # cgroupfs: /docker/ + # + # In a Kubernetes pod, the cgroup path will look like: + # + # systemd: + # /kubepods.slice/kubepods-.slice/kubepods--pod.slice/.scope + # cgroupfs: + # /kubepods//pod/ + + directory, container_id = os.path.split(cgroup_path) + if container_id.endswith(SYSTEMD_SCOPE_SUFFIX): + container_id = container_id[: -len(SYSTEMD_SCOPE_SUFFIX)] + if "-" in container_id: + container_id = container_id.split("-", 1)[1] + kubepods_match = kubepods_regexp.match(directory) + if kubepods_match: + pod_id = kubepods_match.group(1) + if not pod_id: + pod_id = kubepods_match.group(2) + return {"container": {"id": container_id}, "pod": {"uid": pod_id}} + elif container_id_regexp.match(container_id): + return {"container": {"id": container_id}} diff --git a/elasticapm/utils/docker.py b/elasticapm/utils/docker.py deleted file mode 100644 index e83eb92ea..000000000 --- a/elasticapm/utils/docker.py +++ /dev/null @@ -1,57 +0,0 @@ -import os -import re - -DOCKER_ID_PATH = "/proc/self/cgroup" - - -def get_docker_metadata(): - """ - Reads docker/kubernetes metadata (container id, pod id) from /proc/self/cgroup - - The result is a nested dictionary with the detected IDs, e.g. - - { - "container": {"id": "2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63"}, - "pod": {"uid": "90d81341_92de_11e7_8cf2_507b9d4141fa"} - } - - :return: a dictionary with the detected ids or {} - """ - if not os.path.exists(DOCKER_ID_PATH): - return {} - with open(DOCKER_ID_PATH) as f: - return parse_cgroups(f) or {} - - -def parse_cgroups(filehandle): - """ - Reads lines from a file handle and tries to parse docker container IDs and kubernetes Pod IDs. - - See tests.utils.docker_tests.test_cgroup_parsing for a set of test cases - - :param filehandle: - :return: nested dictionary or None - """ - for line in filehandle: - try: - if "docker" not in line and "kubepods" not in line: - continue - parts = line.split(":") - for part in parts: - if "docker" in part: - container_id = part.split("/")[-1] - # older docker versions prepend "docker-" and append ".scope" - if "-" in container_id: - container_id = re.split("[.-]", container_id)[1] - return {"container": {"id": container_id}} - elif "kubepods" in part: - pod_id, container_id = part.split("/")[-2:] - if "pod" in pod_id: - pod_id = pod_id[pod_id.rindex("pod") + 3 :] - if pod_id.endswith(".slice"): - pod_id = pod_id[:-6] - if container_id.endswith(".scope"): - container_id = re.split("[.-]", container_id)[1] - return {"container": {"id": container_id}, "pod": {"uid": pod_id}} - except IndexError: - pass diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index 76e91f550..b153fdc8b 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -40,7 +40,7 @@ def test_process_info(elasticapm_client): def test_system_info(elasticapm_client): # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mocked: + with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mocked: mocked.return_value = {} system_info = elasticapm_client.get_system_info() assert {"hostname", "architecture", "platform"} == set(system_info.keys()) @@ -48,7 +48,7 @@ def test_system_info(elasticapm_client): def test_docker_kubernetes_system_info(elasticapm_client): # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata, mock.patch( + with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata, mock.patch( "socket.gethostname" ) as mock_gethostname: mock_metadata.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} @@ -71,7 +71,7 @@ def test_docker_kubernetes_system_info_from_environ(): # initialize agent only after overriding environment elasticapm_client = Client() # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata: + with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata: mock_metadata.return_value = {} system_info = elasticapm_client.get_system_info() assert "kubernetes" in system_info @@ -95,7 +95,7 @@ def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(): # initialize agent only after overriding environment elasticapm_client = Client() # mock docker/kubernetes data here to get consistent behavior if test is run in docker - with mock.patch("elasticapm.utils.docker.get_docker_metadata") as mock_metadata, mock.patch( + with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata, mock.patch( "socket.gethostname" ) as mock_gethostname: mock_metadata.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}} diff --git a/tests/utils/docker_tests.py b/tests/utils/cgroup_tests.py similarity index 92% rename from tests/utils/docker_tests.py rename to tests/utils/cgroup_tests.py index c37f19880..acfe14f4e 100644 --- a/tests/utils/docker_tests.py +++ b/tests/utils/cgroup_tests.py @@ -1,7 +1,7 @@ import mock import pytest -from elasticapm.utils import compat, docker +from elasticapm.utils import cgroup, compat @pytest.mark.parametrize( @@ -33,5 +33,5 @@ ) def test_cgroup_parsing(test_input, expected): f = compat.StringIO(test_input) - result = docker.parse_cgroups(f) - assert expected == result + result = cgroup.parse_cgroups(f) + assert result == expected From aa72770a73bda8146db25f607184a0ef31666b0f Mon Sep 17 00:00:00 2001 From: Gil Raphaelli Date: Wed, 2 Jan 2019 18:10:34 +0100 Subject: [PATCH 7/7] added test for hostname detection if k8s metadata is set via environment --- tests/client/client_tests.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index b153fdc8b..840d9e1fe 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -69,7 +69,7 @@ def test_docker_kubernetes_system_info(elasticapm_client): ) def test_docker_kubernetes_system_info_from_environ(): # initialize agent only after overriding environment - elasticapm_client = Client() + elasticapm_client = Client(metrics_interval="0ms") # mock docker/kubernetes data here to get consistent behavior if test is run in docker with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata: mock_metadata.return_value = {} @@ -93,7 +93,7 @@ def test_docker_kubernetes_system_info_from_environ(): ) def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(): # initialize agent only after overriding environment - elasticapm_client = Client() + elasticapm_client = Client(metrics_interval="0ms") # mock docker/kubernetes data here to get consistent behavior if test is run in docker with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata, mock.patch( "socket.gethostname" @@ -110,6 +110,21 @@ def test_docker_kubernetes_system_info_from_environ_overrides_cgroups(): assert system_info["container"] == {"id": "123"} +@mock.patch.dict("os.environ", {"KUBERNETES_NAMESPACE": "namespace"}) +def test_docker_kubernetes_system_info_except_hostname_from_environ(): + # initialize agent only after overriding environment + elasticapm_client = Client(metrics_interval="0ms") + # mock docker/kubernetes data here to get consistent behavior if test is run in docker + with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mock_metadata, mock.patch( + "socket.gethostname" + ) as mock_gethostname: + mock_metadata.return_value = {} + mock_gethostname.return_value = "foo" + system_info = elasticapm_client.get_system_info() + assert "kubernetes" in system_info + assert system_info["kubernetes"] == {"pod": {"name": "foo"}, "namespace": "namespace"} + + def test_config_by_environment(): with mock.patch.dict("os.environ", {"ELASTIC_APM_SERVICE_NAME": "envapp", "ELASTIC_APM_SECRET_TOKEN": "envtoken"}): client = Client(metrics_interval="0ms")