From cbb16300424bc7f175a49cd270ee4ced7e33e92e Mon Sep 17 00:00:00 2001 From: Boris Pavlovic Date: Tue, 10 Jun 2014 16:29:00 +0400 Subject: [PATCH] Simplify notifer API Do not use java style type checks, that allows us to drop dependency in oslo.messaging to osprofiler and simplify code. As well this change splits notifier with notification parsing. Add missing unit tests (coverage of notifier & parsers 100%) Change-Id: I5a3cb1582df08e7f98ad643909db6f130104a0e1 --- osprofiler/notifier.py | 130 +++---------------- osprofiler/parsers/__init__.py | 0 osprofiler/parsers/ceilometer.py | 117 +++++++++++++++++ osprofiler/profiler.py | 3 +- tests/parsers/__init__.py | 0 tests/parsers/test_ceilometer.py | 216 +++++++++++++++++++++++++++++++ tests/test_notifier.py | 36 ++++++ tests/test_profiler.py | 17 +-- 8 files changed, 392 insertions(+), 127 deletions(-) create mode 100644 osprofiler/parsers/__init__.py create mode 100644 osprofiler/parsers/ceilometer.py create mode 100644 tests/parsers/__init__.py create mode 100644 tests/parsers/test_ceilometer.py create mode 100644 tests/test_notifier.py diff --git a/osprofiler/notifier.py b/osprofiler/notifier.py index 48488ba..f997007 100644 --- a/osprofiler/notifier.py +++ b/osprofiler/notifier.py @@ -13,131 +13,33 @@ # License for the specific language governing permissions and limitations # under the License. -import datetime + +def noop_notifier(info): + """Do nothing on notification.""" + pass -class Notifier(object): - """Base notifier that should be implemented by every service that would - like to use profiler lib. - """ - - def notify(self, payload): - pass +# NOTE(boris-42): By default we are using noop notifier. +__notifier = noop_notifier -# NOTE(boris-42): By default we will use base Notfier that does nothing. -__notifier = Notifier() +def notify(info): + global __notifier + + __notifier(info) def get_notifier(): + """Returns notifier callable.""" return __notifier def set_notifier(notifier): - """This method should be called by service that use profiler with proper - instance of notfier. + """Service that are going to use profiler should set callable notifier. + + Callable notifier is instance of callable object, that accept exactly + one argument "info". "info" - is dictionary of values that contains + profiling information. """ - if not isinstance(notifier, Notifier): - raise TypeError("notifier should be instance of subclass of " - "notfier.Notifer") global __notifier __notifier = notifier - - -def _build_tree(nodes): - """Builds the tree (forest) data structure based on the list of nodes. - - Works in O(n). - - :param nodes: list of nodes, where each node is a dictionary with fields - "parent_id", "trace_id", "info" - :returns: list of top level ("root") nodes in form of dictionaries, - each containing the "info" and "children" fields, where - "children" is the list of child nodes ("children" will be - empty for leafs) - """ - - tree = [] - - # 1st pass through nodes - populating the cache with existing nodes - for trace_id in nodes: - nodes[trace_id]["children"] = [] - - # 2nd pass through nodes - calculating parent-child relationships - for trace_id, node in nodes.iteritems(): - if node["parent_id"] in nodes: - nodes[node["parent_id"]]["children"].append(nodes[trace_id]) - else: - tree.append(nodes[trace_id]) # no parent => top-level node - - for node in nodes: - nodes[node]["children"].sort(key=lambda x: x["info"]["started"]) - - return sorted(tree, key=lambda x: x["info"]["started"]) - - -def parse_notifications(notifications): - """Parse & builds tree structure from list of ceilometer notifications.""" - - result = {} - started_at = 0 - finished_at = 0 - - for n in notifications: - meta = n["metadata"] - key = meta["trace_id"] - - if key not in result: - result[key] = { - "info": { - "service": meta["event_type"].split(".", 1)[1], - "host": meta["host"], - "name": meta["name"].split("-")[0] - }, - "parent_id": meta["parent_id"], - "trace_id": meta["trace_id"] - } - - skip_keys = ["base_id", "trace_id", "parent_id", "name", "host", - "event_type"] - - for k, v in meta.iteritems(): - if k not in skip_keys: - result[key]["info"][k] = v - - timestamp = datetime.datetime.strptime(n["timestamp"], - "%Y-%m-%dT%H:%M:%S.%f") - - if meta["name"].endswith("stop"): - result[key]["info"]["finished"] = timestamp - else: - result[key]["info"]["started"] = timestamp - - if not started_at or started_at > timestamp: - started_at = timestamp - - if not finished_at or finished_at < timestamp: - finished_at = timestamp - - def msec(deltatime): - return (int)(deltatime.total_seconds() * 1000) - - for r in result.itervalues(): - # NOTE(boris-42): We are not able to guarantee that ceilometer consumed - # all messages => so we should at make duration 0ms. - if "started" not in r["info"]: - r["info"]["started"] = r["info"]["finished"] - if "finished" not in r["info"]: - r["info"]["finished"] = r["info"]["started"] - - r["info"]["started"] = msec(r["info"]["started"] - started_at) - r["info"]["finished"] = msec(r["info"]["finished"] - started_at) - - return { - "info": { - "name": "total", - "started": 0, - "finished": msec(finished_at - started_at) - }, - "children": _build_tree(result) - } diff --git a/osprofiler/parsers/__init__.py b/osprofiler/parsers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/osprofiler/parsers/ceilometer.py b/osprofiler/parsers/ceilometer.py new file mode 100644 index 0000000..701f20e --- /dev/null +++ b/osprofiler/parsers/ceilometer.py @@ -0,0 +1,117 @@ +# Copyright 2014 Mirantis Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +import datetime + + +def _build_tree(nodes): + """Builds the tree (forest) data structure based on the list of nodes. + + Works in O(n). + + :param nodes: list of nodes, where each node is a dictionary with fields + "parent_id", "trace_id", "info" + :returns: list of top level ("root") nodes in form of dictionaries, + each containing the "info" and "children" fields, where + "children" is the list of child nodes ("children" will be + empty for leafs) + """ + + tree = [] + + for trace_id, node in nodes.iteritems(): + node.setdefault("children", []) + parent_id = node["parent_id"] + if parent_id in nodes: + nodes[parent_id].setdefault("children", []) + nodes[parent_id]["children"].append(nodes[trace_id]) + else: + tree.append(node) # no parent => top-level node + + for node in nodes: + nodes[node]["children"].sort(key=lambda x: x["info"]["started"]) + + return sorted(tree, key=lambda x: x["info"]["started"]) + + +def parse_notifications(notifications): + """Parse & builds tree structure from list of ceilometer notifications.""" + + result = {} + started_at = 0 + finished_at = 0 + + for n in notifications: + meta = n["metadata"] + key = meta["trace_id"] + + if key not in result: + result[key] = { + "info": { + "service": meta["event_type"].split(".", 1)[1], + "host": meta["host"], + "name": meta["name"].split("-")[0] + }, + "parent_id": meta["parent_id"], + "trace_id": meta["trace_id"] + } + + skip_keys = ["base_id", "trace_id", "parent_id", "name", "host", + "event_type"] + + for k, v in meta.iteritems(): + if k not in skip_keys: + result[key]["info"][k] = v + + timestamp = datetime.datetime.strptime(n["timestamp"], + "%Y-%m-%dT%H:%M:%S.%f") + + if meta["name"].endswith("stop"): + result[key]["info"]["finished"] = timestamp + else: + result[key]["info"]["started"] = timestamp + + if not started_at or started_at > timestamp: + started_at = timestamp + + if not finished_at or finished_at < timestamp: + finished_at = timestamp + + def msec(dt): + # NOTE(boris-42): Unfortunately this is the simplest way that works in + # py26 and py27 + microsec = (dt.microseconds + (dt.seconds + dt.days * 24 * 3600) * 1e6) + return (int)(microsec / 1000.0) + + for r in result.itervalues(): + # NOTE(boris-42): We are not able to guarantee that ceilometer consumed + # all messages => so we should at make duration 0ms. + if "started" not in r["info"]: + r["info"]["started"] = r["info"]["finished"] + if "finished" not in r["info"]: + r["info"]["finished"] = r["info"]["started"] + + r["info"]["started"] = msec(r["info"]["started"] - started_at) + r["info"]["finished"] = msec(r["info"]["finished"] - started_at) + + return { + "info": { + "name": "total", + "started": 0, + "finished": msec(finished_at - started_at) + }, + "children": _build_tree(result) + } diff --git a/osprofiler/profiler.py b/osprofiler/profiler.py index d5af832..62cdd71 100644 --- a/osprofiler/profiler.py +++ b/osprofiler/profiler.py @@ -64,7 +64,6 @@ def stop(info=None): class Profiler(object): def __init__(self, base_id=None, parent_id=None, hmac_key=None): - self.notifier = notifier.get_notifier() self.hmac_key = hmac_key if not base_id: base_id = str(uuid.uuid4()) @@ -120,4 +119,4 @@ class Profiler(object): if info: payload['info'] = info - self.notifier.notify(payload) + notifier.notify(payload) diff --git a/tests/parsers/__init__.py b/tests/parsers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/parsers/test_ceilometer.py b/tests/parsers/test_ceilometer.py new file mode 100644 index 0000000..4c7f374 --- /dev/null +++ b/tests/parsers/test_ceilometer.py @@ -0,0 +1,216 @@ +# Copyright 2014 Mirantis Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from osprofiler.parsers import ceilometer + +from tests import test + + +class CeilometerParserTestCase(test.TestCase): + + def test_build_empty_tree(self): + self.assertEqual(ceilometer._build_tree({}), []) + + def test_build_complex_tree(self): + test_input = { + "2": {"parent_id": "0", "trace_id": "2", "info": {"started": 1}}, + "1": {"parent_id": "0", "trace_id": "1", "info": {"started": 0}}, + "21": {"parent_id": "2", "trace_id": "21", "info": {"started": 6}}, + "22": {"parent_id": "2", "trace_id": "22", "info": {"started": 7}}, + "11": {"parent_id": "1", "trace_id": "11", "info": {"started": 1}}, + "113":{"parent_id": "11", "trace_id": "113", + "info": {"started": 3}}, + "112": {"parent_id": "11", "trace_id": "112", + "info": {"started": 2}}, + "114": {"parent_id": "11", "trace_id": "114", + "info": {"started": 5}} + } + + expected_output = [ + { + "parent_id": "0", + "trace_id": "1", + "info": {"started": 0}, + "children": [ + { + "parent_id": "1", + "trace_id": "11", + "info": {"started": 1}, + "children": [ + {"parent_id": "11", "trace_id": "112", + "info": {"started": 2}, "children": []}, + {"parent_id": "11", "trace_id": "113", + "info": {"started": 3}, "children": []}, + {"parent_id": "11", "trace_id": "114", + "info": {"started": 5}, "children": []} + ] + } + ] + }, + { + "parent_id": "0", + "trace_id": "2", + "info": {"started": 1}, + "children": [ + {"parent_id": "2", "trace_id": "21", + "info": {"started": 6}, "children": []}, + {"parent_id": "2", "trace_id": "22", + "info": {"started": 7}, "children": []} + ] + } + ] + + self.assertEqual(ceilometer._build_tree(test_input), expected_output) + + def test_parse_notifications(self): + samples = [ + { + "id": "896f5e52-d4c9-11e3-a117-46c0b36ac153", + "metadata": { + "base_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "event_type": "profiler.nova", + "host": "osapi_compute.0.0.0.0", + "name": "WSGI-stop", + "parent_id": "82281b35-63aa-45fc-8578-5a32a66370ab", + "trace_id": "837eb0bd-323a-4e3f-b223-3be78ad86aab" + }, + "meter": "WSGI-stop", + "project_id": None, + "recorded_at": "2014-05-06T02:53:03.110724", + "resource_id": "profiler", + "source": "openstack", + "timestamp": "2014-05-06T02:52:59.357020", + "type": "gauge", + "unit": "sample", + "user_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "volume": 1.0 + }, + { + "id": "895043a0-d4c9-11e3-a117-46c0b36ac153", + "metadata": { + "base_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "event_type": "profiler.nova", + "host": "osapi_compute.0.0.0.0", + "name": "WSGI-start", + "parent_id": "82281b35-63aa-45fc-8578-5a32a66370ab", + "trace_id": "837eb0bd-323a-4e3f-b223-3be78ad86aab" + }, + "meter": "WSGI-start", + "project_id": None, + "recorded_at": "2014-05-06T02:53:03.020620", + "resource_id": "profiler", + "source": "openstack", + "timestamp": "2014-05-06T02:52:59.225552", + "type": "gauge", + "unit": "sample", + "user_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "volume": 1.0 + }, + + { + "id": "89558414-d4c9-11e3-a117-46c0b36ac153", + "metadata": { + "base_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "event_type": "profiler.nova", + "host": "osapi_compute.0.0.0.0", + "info.db:multiparams": "(immutabledict({}),)", + "info.db:params": "{}", + "name": "nova.db-start", + "parent_id": "837eb0bd-323a-4e3f-b223-3be78ad86aab", + "trace_id": "f8ab042e-1085-4df2-9f3a-cfb6390b8090" + }, + "meter": "nova.db-start", + "project_id": None, + "recorded_at": "2014-05-06T02:53:03.038692", + "resource_id": "profiler", + "source": "openstack", + "timestamp": "2014-05-06T02:52:59.273422", + "type": "gauge", + "unit": "sample", + "user_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "volume": 1.0 + }, + { + "id": "892d3018-d4c9-11e3-a117-46c0b36ac153", + "metadata": { + "base_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "event_type": "profiler.generic", + "host": "nova-conductor.ubuntu", + "name": "nova.db-stop", + "parent_id": "aad4748f-99d5-45c8-be0a-4025894bb3db", + "trace_id": "8afee05d-0ad2-4515-bd03-db0f2d30eed0" + }, + "meter": "nova.db-stop", + "project_id": None, + "recorded_at": "2014-05-06T02:53:02.894015", + "resource_id": "profiler", + "source": "openstack", + "timestamp": "2014-05-06T02:53:00.473201", + "type": "gauge", + "unit": "sample", + "user_id": "f5587500-07d1-41a0-b434-525d3c28ac49", + "volume": 1.0 + } + ] + + excepted = { + "info": { + "finished": 1247, + "name": "total", + "started": 0 + }, + "children": [ + { + "info": { + "finished": 131, + "host": "osapi_compute.0.0.0.0", + "name": "WSGI", + "service": "nova", + "started": 0 + }, + "parent_id": "82281b35-63aa-45fc-8578-5a32a66370ab", + "trace_id": "837eb0bd-323a-4e3f-b223-3be78ad86aab", + "children": [{ + "children": [], + "info": { + "finished": 47, + "host": "osapi_compute.0.0.0.0", + "info.db:multiparams": "(immutabledict({}),)", + "info.db:params": "{}", + "name": "nova.db", + "service": "nova", + "started": 47 + }, + + "parent_id": "837eb0bd-323a-4e3f-b223-3be78ad86aab", + "trace_id": "f8ab042e-1085-4df2-9f3a-cfb6390b8090" + }] + }, + { + "children": [], + "info": { + "finished": 1247, + "host": "nova-conductor.ubuntu", + "name": "nova.db", + "service": "generic", + "started": 1247 + }, + "parent_id": "aad4748f-99d5-45c8-be0a-4025894bb3db", + "trace_id": "8afee05d-0ad2-4515-bd03-db0f2d30eed0" + } + ] + } + + self.assertEqual(ceilometer.parse_notifications(samples), excepted) diff --git a/tests/test_notifier.py b/tests/test_notifier.py new file mode 100644 index 0000000..2d0bf46 --- /dev/null +++ b/tests/test_notifier.py @@ -0,0 +1,36 @@ +# Copyright 2014 Mirantis Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from osprofiler import notifier + +from tests import test + + +class NotifierTestCase(test.TestCase): + + def tearDown(self): + notifier.__notifier = notifier.noop_notifier + super(NotifierTestCase, self).tearDown() + + def test_set_notifier(self): + + def test(info): + pass + + notifier.set_notifier(test) + self.assertEqual(notifier.get_notifier(), test) + + def test_get_default_notifier(self): + self.assertEqual(notifier.get_notifier(), notifier.noop_notifier) diff --git a/tests/test_profiler.py b/tests/test_profiler.py index c3612a5..04e2a5b 100644 --- a/tests/test_profiler.py +++ b/tests/test_profiler.py @@ -84,11 +84,9 @@ class ProfilerTestCase(test.TestCase): self.assertEqual(prof.get_id(), "43") @mock.patch("osprofiler.profiler.uuid.uuid4") - @mock.patch("osprofiler.profiler.notifier.get_notifier") - def test_profiler_start(self, mock_get_notfier, mock_uuid4): + @mock.patch("osprofiler.profiler.notifier.notify") + def test_profiler_start(self, mock_notify, mock_uuid4): mock_uuid4.return_value = "44" - notifier = mock.MagicMock() - mock_get_notfier.return_value = notifier info = {"some": "info"} payload = { @@ -102,13 +100,10 @@ class ProfilerTestCase(test.TestCase): prof = profiler.Profiler(base_id="1", parent_id="2") prof.start("test", info=info) - notifier.notify.assert_called_once_with(payload) - - @mock.patch("osprofiler.profiler.notifier.get_notifier") - def test_profiler_stop(self, mock_get_notfier): - notifier = mock.MagicMock() - mock_get_notfier.return_value = notifier + mock_notify.assert_called_once_with(payload) + @mock.patch("osprofiler.profiler.notifier.notify") + def test_profiler_stop(self, mock_notify): prof = profiler.Profiler(base_id="1", parent_id="2") prof._trace_stack.append("44") prof._name.append("abc") @@ -124,7 +119,7 @@ class ProfilerTestCase(test.TestCase): "info": info } - notifier.notify.assert_called_once_with(payload) + mock_notify.assert_called_once_with(payload) self.assertEqual(len(prof._name), 0) self.assertEqual(prof._trace_stack, collections.deque(["1", "2"]))