From ac1ec83ecdff7da9a8c385c653e6494453ab733f Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Wed, 24 Jan 2024 00:55:01 +0000 Subject: [PATCH 1/5] feat(otel): update otel support - fix metric processor to extend plugin - add otel metrics plugin - fix dependency definitions in toml --- Makefile | 6 + pyproject.toml | 11 +- src/deep/api/plugin/__init__.py | 1 + src/deep/api/plugin/metric/__init__.py | 4 +- src/deep/api/plugin/metric/otel_metrics.py | 133 ++++++++++++++++++ .../api/plugin/metric/prometheus_metrics.py | 7 +- src/deep/api/plugin/otel.py | 2 +- 7 files changed, 153 insertions(+), 11 deletions(-) create mode 100644 src/deep/api/plugin/metric/otel_metrics.py diff --git a/Makefile b/Makefile index 07fe739..80f385c 100644 --- a/Makefile +++ b/Makefile @@ -33,6 +33,12 @@ lint: .PHONY: flake flake: lint +.PHONY: build +build: + rm -Rf $(ROOT_DIR)/dist + + python -m build $(ROOT_DIR) + .PHONY: check-python-vars check-python-vars: ifndef TWINE_USER diff --git a/pyproject.toml b/pyproject.toml index fcd34db..5a03271 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,12 @@ classifiers = [ ] dynamic = [ "version", - "dependencies" +] + +dependencies = [ + "grpcio>=1.51.3", + "deep-proto>=1.0.5", + "protobuf>=3.20.3" ] [tool.hatch.build.targets.sdist] @@ -44,10 +49,6 @@ packages = ["src/deep"] [tool.hatch.version] path = "src/deep/version.py" -# read dependencies from reuirements.txt -[tool.setuptools.dynamic] -dependencies = {file = ["requirements.txt"]} - [tool.pytest.ini_options] pythonpath = [ "./src", diff --git a/src/deep/api/plugin/__init__.py b/src/deep/api/plugin/__init__.py index 02faf36..b7569d3 100644 --- a/src/deep/api/plugin/__init__.py +++ b/src/deep/api/plugin/__init__.py @@ -33,6 +33,7 @@ 'deep.api.plugin.otel.OTelPlugin', 'deep.api.plugin.python.PythonPlugin', 'deep.api.plugin.metric.prometheus_metrics.PrometheusPlugin', + 'deep.api.plugin.metric.otel_metrics.OTelMetrics', ] """System provided default plugins.""" diff --git a/src/deep/api/plugin/metric/__init__.py b/src/deep/api/plugin/metric/__init__.py index 395d70a..db788b2 100644 --- a/src/deep/api/plugin/metric/__init__.py +++ b/src/deep/api/plugin/metric/__init__.py @@ -21,8 +21,10 @@ import abc from typing import Dict +from deep.api.plugin import Plugin -class MetricProcessor(abc.ABC): + +class MetricProcessor(Plugin, abc.ABC): """Metric processor connects Deep to a metric provider.""" @abc.abstractmethod diff --git a/src/deep/api/plugin/metric/otel_metrics.py b/src/deep/api/plugin/metric/otel_metrics.py new file mode 100644 index 0000000..5b8d756 --- /dev/null +++ b/src/deep/api/plugin/metric/otel_metrics.py @@ -0,0 +1,133 @@ +# Copyright (C) 2024 Intergral GmbH +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Handling for OTEL metric support.""" +import threading +from typing import Dict + +import deep.logging +from deep.api.plugin import DidNotEnable +from deep.api.plugin.metric import MetricProcessor + +try: + from opentelemetry.metrics import get_meter, Counter, ObservableGauge, Histogram, UpDownCounter +except ImportError as e: + raise DidNotEnable("opentelemetry.metrics is not installed", e) + + +class OTelMetrics(MetricProcessor): + """ + Metric processor for otel. + + Separate from OTEL Plugin for spans, as you can have one without the other. + """ + + def __init__(self, config): + """Create new plugin.""" + super().__init__("OTelMetrics", config) + self.__cache = {} + self.__lock = threading.Lock() + + def __check_cache(self, name, type_name, from_default): + cache_key = f'{name}_{type_name}' + if cache_key in self.__cache: + return self.__cache[cache_key] + default = from_default() + self.__cache[cache_key] = default + return default + + def counter(self, name: str, labels: Dict[str, str], namespace: str, help_string: str, unit: str, value: float): + """ + Create a counter type value in the provider. + + :param name: the metric name + :param labels: the metric labels + :param namespace: the metric namespace + :param help_string: the metric help string + :param unit: the metric unit + :param value: the metric value + """ + try: + with self.__lock: + counter: Counter + counter = self.__check_cache(name, 'counter', + lambda: get_meter('deep').create_counter(f'{namespace}_{name}', unit, + help_string)) + counter.add(value, labels) + except Exception: + deep.logging.exception(f"Error registering metric counter {namespace}_{name}") + + def gauge(self, name: str, labels: Dict[str, str], namespace: str, help_string: str, unit: str, value: float): + """ + Create a gauge type value in the provider. + + :param name: the metric name + :param labels: the metric labels + :param namespace: the metric namespace + :param help_string: the metric help string + :param unit: the metric unit + :param value: the metric value + """ + try: + with self.__lock: + gauge: UpDownCounter + gauge = self.__check_cache(name, 'gauge', + lambda: get_meter('deep').create_up_down_counter(f'{namespace}_{name}', + unit, help_string)) + gauge.add(value, labels) + except Exception: + deep.logging.exception(f"Error registering metric histogram {namespace}_{name}") + + def histogram(self, name: str, labels: Dict[str, str], namespace: str, help_string: str, unit: str, value: float): + """ + Create a histogram type value in the provider. + + :param name: the metric name + :param labels: the metric labels + :param namespace: the metric namespace + :param help_string: the metric help string + :param unit: the metric unit + :param value: the metric value + """ + try: + with self.__lock: + histogram: Histogram + histogram = self.__check_cache(name, 'histogram', + lambda: get_meter('deep').create_histogram(f'{namespace}_{name}', unit, + help_string)) + histogram.record(value, labels) + except Exception: + deep.logging.exception(f"Error registering metric histogram {namespace}_{name}") + + def summary(self, name: str, labels: Dict[str, str], namespace: str, help_string: str, unit: str, value: float): + """ + Create a summary type value in the provider. + + :param name: the metric name + :param labels: the metric labels + :param namespace: the metric namespace + :param help_string: the metric help string + :param unit: the metric unit + :param value: the metric value + """ + try: + with self.__lock: + histogram: Histogram + histogram = self.__check_cache(name, 'summary', + lambda: get_meter('deep').create_histogram(f'{namespace}_{name}', unit, + help_string)) + histogram.record(value, labels) + except Exception: + deep.logging.exception(f"Error registering metric summary {namespace}_{name}") diff --git a/src/deep/api/plugin/metric/prometheus_metrics.py b/src/deep/api/plugin/metric/prometheus_metrics.py index 3e21a9e..cc4b4e1 100644 --- a/src/deep/api/plugin/metric/prometheus_metrics.py +++ b/src/deep/api/plugin/metric/prometheus_metrics.py @@ -19,16 +19,16 @@ from typing import Dict import deep.logging -from deep.api.plugin import DidNotEnable, Plugin +from deep.api.plugin import DidNotEnable from deep.api.plugin.metric import MetricProcessor try: from prometheus_client import Summary, Counter, REGISTRY, Histogram, Gauge except ImportError as e: - raise DidNotEnable("opentelemetry is not installed", e) + raise DidNotEnable("prometheus_client is not installed", e) -class PrometheusPlugin(Plugin, MetricProcessor): +class PrometheusPlugin(MetricProcessor): """Connect Deep to prometheus.""" def __init__(self, config): @@ -68,7 +68,6 @@ def counter(self, name: str, labels: Dict[str, str], namespace: str, help_string counter.inc(value) except Exception: deep.logging.exception(f"Error registering metric counter {namespace}_{name}") - pass def gauge(self, name: str, labels: Dict[str, str], namespace: str, help_string: str, unit: str, value: float): """ diff --git a/src/deep/api/plugin/otel.py b/src/deep/api/plugin/otel.py index efc2258..d0064ca 100644 --- a/src/deep/api/plugin/otel.py +++ b/src/deep/api/plugin/otel.py @@ -29,7 +29,7 @@ # noinspection PyProtectedMember from opentelemetry.sdk.trace import _Span, TracerProvider except ImportError as e: - raise DidNotEnable("opentelemetry is not installed", e) + raise DidNotEnable("opentelemetry.sdk is not installed", e) class _OtelSpan(Span): From 5bd03a32e6054f41c94fadd8ac1965f3ddb3125a Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Tue, 6 Feb 2024 11:41:50 +0000 Subject: [PATCH 2/5] fix(lint): remove unused import --- src/deep/api/plugin/metric/otel_metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deep/api/plugin/metric/otel_metrics.py b/src/deep/api/plugin/metric/otel_metrics.py index 5b8d756..a3d42a4 100644 --- a/src/deep/api/plugin/metric/otel_metrics.py +++ b/src/deep/api/plugin/metric/otel_metrics.py @@ -22,7 +22,7 @@ from deep.api.plugin.metric import MetricProcessor try: - from opentelemetry.metrics import get_meter, Counter, ObservableGauge, Histogram, UpDownCounter + from opentelemetry.metrics import get_meter, Counter, Histogram, UpDownCounter except ImportError as e: raise DidNotEnable("opentelemetry.metrics is not installed", e) From 5592d5e483171090b96378bc00d983d389ca179f Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Tue, 6 Feb 2024 11:43:16 +0000 Subject: [PATCH 3/5] fix(test): fix broken test --- tests/unit_tests/api/plugin/test_plugin.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/api/plugin/test_plugin.py b/tests/unit_tests/api/plugin/test_plugin.py index 32879c8..a1e3a25 100644 --- a/tests/unit_tests/api/plugin/test_plugin.py +++ b/tests/unit_tests/api/plugin/test_plugin.py @@ -34,13 +34,13 @@ def setUp(self): def test_load_plugins(self): plugins = load_plugins(None) self.assertIsNotNone(plugins) - self.assertEqual(3, len(plugins)) + self.assertEqual(4, len(plugins)) def test_handle_bad_plugin(self): plugins = load_plugins(None, [BadPlugin.__qualname__]) - self.assertEqual(3, len(plugins)) + self.assertEqual(4, len(plugins)) plugins = load_plugins(None, [BadPlugin.__module__ + '.' + BadPlugin.__name__]) - self.assertEqual(3, len(plugins)) + self.assertEqual(4, len(plugins)) From 08e052aa23c703aec09ab4f6f4eabab144c06718 Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Tue, 6 Feb 2024 12:48:18 +0000 Subject: [PATCH 4/5] fix(metrics): add additional tests --- .../api/plugin/metric/prometheus_metrics.py | 4 + .../api/plugin/metrics/test_otel_metrics.py | 117 ++++++++++++++++++ .../plugin/metrics/test_prometheus_metrics.py | 44 ++++++- 3 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/api/plugin/metrics/test_otel_metrics.py diff --git a/src/deep/api/plugin/metric/prometheus_metrics.py b/src/deep/api/plugin/metric/prometheus_metrics.py index cc4b4e1..14cc1c5 100644 --- a/src/deep/api/plugin/metric/prometheus_metrics.py +++ b/src/deep/api/plugin/metric/prometheus_metrics.py @@ -144,6 +144,10 @@ def summary(self, name: str, labels: Dict[str, str], namespace: str, help_string deep.logging.exception(f"Error registering metric summary {namespace}_{name}") pass + @property + def _cache(self): + return self.__cache + def shutdown(self): """Clean up and shutdown the plugin.""" self.clear() diff --git a/tests/unit_tests/api/plugin/metrics/test_otel_metrics.py b/tests/unit_tests/api/plugin/metrics/test_otel_metrics.py new file mode 100644 index 0000000..e84d1bc --- /dev/null +++ b/tests/unit_tests/api/plugin/metrics/test_otel_metrics.py @@ -0,0 +1,117 @@ +# Copyright (C) 2024 Intergral GmbH +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +import unittest + +from mockito import when, mock, verify +from opentelemetry.metrics import set_meter_provider, MeterProvider + +import deep.logging +from deep.api.plugin.metric.otel_metrics import OTelMetrics + + +class TestOtelMetrics(unittest.TestCase): + mock_provider = mock(MeterProvider) + + @classmethod + def setUpClass(cls): + deep.logging.init() + + set_meter_provider(cls.mock_provider) + + def setUp(self): + self.deep_provider = mock() + when(self.mock_provider).get_meter('deep', '').thenReturn(self.deep_provider) + when(self.mock_provider).get_meter('deep', '', None).thenReturn(self.deep_provider) + + def test_post_counter_twice(self): + metric = mock() + when(self.deep_provider).create_counter('test_counter', "unit", "help").thenReturn(metric) + metrics = OTelMetrics(None) + metrics.counter("counter", {}, "test", "help", "unit", 1) + metrics.counter("counter", {}, "test", "help", "unit", 1) + + verify(self.deep_provider, 1).create_counter('test_counter', "unit", "help") + verify(metric, 2).add(1, {}) + + def test_post_counter(self): + metric = mock() + when(self.deep_provider).create_counter('test_counter', "unit", "help").thenReturn(metric) + metrics = OTelMetrics(None) + metrics.counter("counter", {}, "test", "help", "unit", 1) + + verify(self.deep_provider, 1).create_counter('test_counter', "unit", "help") + + verify(metric).add(1, {}) + + def test_post_histogram(self): + metric = mock() + when(self.deep_provider).create_histogram('test_histogram', "unit", "help").thenReturn(metric) + metrics = OTelMetrics(None) + metrics.histogram("histogram", {}, "test", "help", "unit", 1) + + verify(metric).record(1, {}) + + def test_post_gauge(self): + metric = mock() + when(self.deep_provider).create_up_down_counter('test_gauge', "unit", "help").thenReturn(metric) + metrics = OTelMetrics(None) + metrics.gauge("gauge", {}, "test", "help", "unit", 1) + + verify(metric).add(1, {}) + + def test_post_summary(self): + metric = mock() + when(self.deep_provider).create_histogram('test_summary', "unit", "help").thenReturn(metric) + metrics = OTelMetrics(None) + metrics.summary("summary", {}, "test", "help", "unit", 1) + + verify(metric).record(1, {}) + + def test_post_counter_error(self): + metric = mock() + when(self.deep_provider).create_counter('test_counter', "unit", "help").thenRaise(Exception("test otel error")) + metrics = OTelMetrics(None) + metrics.counter("counter", {}, "test", "help", "unit", 1) + + verify(self.deep_provider, 1).create_counter('test_counter', "unit", "help") + + verify(metric, 0).add(1, {}) + + def test_post_histogram_error(self): + metric = mock() + when(self.deep_provider).create_histogram('test_histogram', "unit", "help").thenRaise( + Exception("test otel error")) + metrics = OTelMetrics(None) + metrics.histogram("histogram", {}, "test", "help", "unit", 1) + + verify(metric, 0).record(1, {}) + + def test_post_gauge_error(self): + metric = mock() + when(self.deep_provider).create_up_down_counter('test_gauge', "unit", "help").thenRaise( + Exception("test otel error")) + metrics = OTelMetrics(None) + metrics.gauge("gauge", {}, "test", "help", "unit", 1) + + verify(metric, 0).add(1, {}) + + def test_post_summary_error(self): + metric = mock() + when(self.deep_provider).create_histogram('test_summary', "unit", "help").thenRaise( + Exception("test otel error")) + metrics = OTelMetrics(None) + metrics.summary("summary", {}, "test", "help", "unit", 1) + + verify(metric, 0).record(1, {}) diff --git a/tests/unit_tests/api/plugin/metrics/test_prometheus_metrics.py b/tests/unit_tests/api/plugin/metrics/test_prometheus_metrics.py index 8234ffc..5b54476 100644 --- a/tests/unit_tests/api/plugin/metrics/test_prometheus_metrics.py +++ b/tests/unit_tests/api/plugin/metrics/test_prometheus_metrics.py @@ -29,10 +29,52 @@ def tearDown(self): def test_counter(self): self.plugin.counter("test", {}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) def test_counter_with_labels(self): self.plugin.counter("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) - def test_duplicate_registration(self): + def test_duplicate_counter_registration(self): self.plugin.counter("test_other", {}, "deep", "", "", 123) self.plugin.counter("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_gauge(self): + self.plugin.gauge("test", {}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_gauge_with_labels(self): + self.plugin.gauge("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_duplicate_gauge_registration(self): + self.plugin.gauge("test_other", {}, "deep", "", "", 123) + self.plugin.gauge("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_histogram(self): + self.plugin.histogram("test", {}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_histogram_with_labels(self): + self.plugin.histogram("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_duplicate_histogram_registration(self): + self.plugin.histogram("test_other", {}, "deep", "", "", 123) + self.plugin.histogram("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_summary(self): + self.plugin.summary("test", {}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_summary_with_labels(self): + self.plugin.summary("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) + + def test_duplicate_summary_registration(self): + self.plugin.summary("test_other", {}, "deep", "", "", 123) + self.plugin.summary("test_other", {'value': 'name'}, "deep", "", "", 123) + self.assertEqual(1, len(self.plugin._cache)) From f06bd79ed8a402dd21568980755db92c8b1675e5 Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Tue, 6 Feb 2024 12:57:30 +0000 Subject: [PATCH 5/5] chore(changelog): update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bad074d..972783c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,9 @@ - **[FEATURE]**: feat(plugins): change plugins to allow better customisation [#22](https://github.com/intergral/deep/pull/22) [@Umaaz](https://github.com/Umaaz) - **[FEATURE]**: feat(metrics): initial implementation of metric points [#21](https://github.com/intergral/deep/pull/21) [@Umaaz](https://github.com/Umaaz) - **[FEATURE]**: feat(spans): initial implementation of span points [#25](https://github.com/intergral/deep/pull/25) [@Umaaz](https://github.com/Umaaz) +- **[FEATURE]**: feat(api): add api function to register tracepoint directly [#8](https://github.com/intergral/deep/pull/8) [@Umaaz](https://github.com/Umaaz) - **[ENHANCEMENT]**: enhancement(trigger): change tracepoint handling to use triggers [#16](https://github.com/intergral/deep/pull/16) [@Umaaz](https://github.com/Umaaz) -- **[BUGFIX]**: feat(api): add api function to register tracepoint directly [#8](https://github.com/intergral/deep/pull/8) [@Umaaz](https://github.com/Umaaz) +- **[BUGFIX]**: fix(metrics): correct handling of metrics in otel [#30](https://github.com/intergral/deep/pull/30) [@Umaaz](https://github.com/Umaaz) # 1.0.1 (22/06/2023)