From 5247b67a400ddcd7c33d5f3b687e4569b081c9e8 Mon Sep 17 00:00:00 2001 From: saimedhi Date: Wed, 3 Apr 2024 14:28:01 -0700 Subject: [PATCH 1/4] Introduced service time metrics to opensearch-py client Signed-off-by: saimedhi --- CHANGELOG.md | 1 + dev-requirements.txt | 1 + docs/source/api-ref/metrics.md | 10 ++ opensearchpy/__init__.py | 3 + opensearchpy/client/client.py | 6 +- opensearchpy/connection/http_requests.py | 9 ++ opensearchpy/connection/http_urllib3.py | 10 ++ opensearchpy/metrics/__init__.py | 16 +++ opensearchpy/metrics/metrics.py | 42 ++++++++ opensearchpy/metrics/metrics_events.py | 61 ++++++++++++ opensearchpy/transport.py | 7 +- setup.py | 1 + test_opensearchpy/test_server/test_metrics.py | 98 +++++++++++++++++++ 13 files changed, 263 insertions(+), 2 deletions(-) create mode 100644 docs/source/api-ref/metrics.md create mode 100644 opensearchpy/metrics/__init__.py create mode 100644 opensearchpy/metrics/metrics.py create mode 100644 opensearchpy/metrics/metrics_events.py create mode 100644 test_opensearchpy/test_server/test_metrics.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f46a76e..305daa34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Enhance generator to generate plugins ([#700](https://github.com/opensearch-project/opensearch-py/pull/700)) - Enhance generator to update changelog only if generated code differs from existing ([#684](https://github.com/opensearch-project/opensearch-py/pull/684)) - Added guide for configuring ssl_assert_hostname ([#694](https://github.com/opensearch-project/opensearch-py/pull/694)) +- Introduced `service time` metrics to opensearch-py client ([#716](https://github.com/opensearch-project/opensearch-py/pull/716)) ### Changed - Updated the `get_policy` API in the index_management plugin to allow the policy_id argument as optional ([#633](https://github.com/opensearch-project/opensearch-py/pull/633)) - Updated the `point_in_time.md` guide with examples demonstrating the usage of the new APIs as alternatives to the deprecated ones. ([#661](https://github.com/opensearch-project/opensearch-py/pull/661)) diff --git a/dev-requirements.txt b/dev-requirements.txt index c41d953c..487c1271 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -8,6 +8,7 @@ sphinx_rtd_theme jinja2 pytz deepmerge +Events # No wheels for Python 3.10 yet! numpy; python_version<"3.10" diff --git a/docs/source/api-ref/metrics.md b/docs/source/api-ref/metrics.md new file mode 100644 index 00000000..9c8c6541 --- /dev/null +++ b/docs/source/api-ref/metrics.md @@ -0,0 +1,10 @@ +# metrics + +```{eval-rst} +.. autoclass:: opensearchpy.metrics.metrics.Metrics +``` + +```{eval-rst} +.. autoclass:: opensearchpy.metrics.metrics_events.MetricsEvents +``` + diff --git a/opensearchpy/__init__.py b/opensearchpy/__init__.py index b852272b..f86e18d3 100644 --- a/opensearchpy/__init__.py +++ b/opensearchpy/__init__.py @@ -133,6 +133,7 @@ from .helpers.update_by_query import UpdateByQuery from .helpers.utils import AttrDict, AttrList, DslBase from .helpers.wrappers import Range +from .metrics import Metrics, MetricsEvents from .serializer import JSONSerializer from .transport import Transport @@ -240,6 +241,8 @@ "token_filter", "tokenizer", "__versionstr__", + "Metrics", + "MetricsEvents", ] try: diff --git a/opensearchpy/client/client.py b/opensearchpy/client/client.py index 091bb5e9..adc4b097 100644 --- a/opensearchpy/client/client.py +++ b/opensearchpy/client/client.py @@ -10,6 +10,7 @@ from typing import Any, Optional, Type from opensearchpy.client.utils import _normalize_hosts +from opensearchpy.metrics.metrics import Metrics from opensearchpy.transport import Transport @@ -22,6 +23,7 @@ def __init__( self, hosts: Optional[str] = None, transport_class: Type[Transport] = Transport, + metrics: Optional[Metrics] = None, **kwargs: Any ) -> None: """ @@ -38,4 +40,6 @@ class as kwargs, or a string in the format of ``host[:port]`` which will be :class:`~opensearchpy.Transport` class and, subsequently, to the :class:`~opensearchpy.Connection` instances. """ - self.transport = transport_class(_normalize_hosts(hosts), **kwargs) + self.transport = transport_class( + _normalize_hosts(hosts), metrics=metrics, **kwargs + ) diff --git a/opensearchpy/connection/http_requests.py b/opensearchpy/connection/http_requests.py index 9bf83004..46518f40 100644 --- a/opensearchpy/connection/http_requests.py +++ b/opensearchpy/connection/http_requests.py @@ -36,6 +36,8 @@ except ImportError: REQUESTS_AVAILABLE = False +from opensearchpy.metrics.metrics import Metrics + from ..compat import reraise_exceptions, string_types, urlencode from ..exceptions import ( ConnectionError, @@ -86,8 +88,10 @@ def __init__( http_compress: Any = None, opaque_id: Any = None, pool_maxsize: Any = None, + metrics: Optional[Metrics] = None, **kwargs: Any ) -> None: + self.metrics = metrics if not REQUESTS_AVAILABLE: raise ImproperlyConfigured( "Please install requests to use RequestsHttpConnection." @@ -188,6 +192,8 @@ def perform_request( # type: ignore } send_kwargs.update(settings) try: + if self.metrics is not None: + self.metrics.request_start() response = self.session.send(prepared_request, **send_kwargs) duration = time.time() - start raw_data = response.content.decode("utf-8", "surrogatepass") @@ -207,6 +213,9 @@ def perform_request( # type: ignore if isinstance(e, requests.Timeout): raise ConnectionTimeout("TIMEOUT", str(e), e) raise ConnectionError("N/A", str(e), e) + finally: + if self.metrics is not None: + self.metrics.request_end() # raise warnings if any from the 'Warnings' header. warnings_headers = ( diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index ab9a1a78..21287859 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -34,6 +34,8 @@ from urllib3.exceptions import SSLError as UrllibSSLError from urllib3.util.retry import Retry +from opensearchpy.metrics.metrics import Metrics + from ..compat import reraise_exceptions, urlencode from ..exceptions import ( ConnectionError, @@ -115,8 +117,10 @@ def __init__( ssl_context: Any = None, http_compress: Any = None, opaque_id: Any = None, + metrics: Optional[Metrics] = None, **kwargs: Any ) -> None: + self.metrics = metrics # Initialize headers before calling super().__init__(). self.headers = urllib3.make_headers(keep_alive=True) @@ -268,6 +272,9 @@ def perform_request( if isinstance(self.http_auth, Callable): # type: ignore request_headers.update(self.http_auth(method, full_url, body)) + if self.metrics is not None: + self.metrics.request_start() + response = self.pool.urlopen( method, url, body, retries=Retry(False), headers=request_headers, **kw ) @@ -284,6 +291,9 @@ def perform_request( if isinstance(e, ReadTimeoutError): raise ConnectionTimeout("TIMEOUT", str(e), e) raise ConnectionError("N/A", str(e), e) + finally: + if self.metrics is not None: + self.metrics.request_end() # raise warnings if any from the 'Warnings' header. warning_headers = response.headers.get_all("warning", ()) diff --git a/opensearchpy/metrics/__init__.py b/opensearchpy/metrics/__init__.py new file mode 100644 index 00000000..38946518 --- /dev/null +++ b/opensearchpy/metrics/__init__.py @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. +# +# Modifications Copyright OpenSearch Contributors. See +# GitHub history for details. + +from .metrics import Metrics +from .metrics_events import MetricsEvents + +__all__ = [ + "Metrics", + "MetricsEvents", +] diff --git a/opensearchpy/metrics/metrics.py b/opensearchpy/metrics/metrics.py new file mode 100644 index 00000000..8764976c --- /dev/null +++ b/opensearchpy/metrics/metrics.py @@ -0,0 +1,42 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. +# +# Modifications Copyright OpenSearch Contributors. See +# GitHub history for details. + +from abc import ABC, abstractmethod +from typing import Optional + + +class Metrics(ABC): + """ + The Metrics class defines methods and properties for managing + request metrics, including start time, end time, and service time, + serving as a blueprint for concrete implementations. + """ + + @abstractmethod + def request_start(self) -> None: + pass + + @abstractmethod + def request_end(self) -> None: + pass + + @property + @abstractmethod + def start_time(self) -> Optional[float]: + pass + + @property + @abstractmethod + def end_time(self) -> Optional[float]: + pass + + @property + @abstractmethod + def service_time(self) -> Optional[float]: + pass diff --git a/opensearchpy/metrics/metrics_events.py b/opensearchpy/metrics/metrics_events.py new file mode 100644 index 00000000..994d5b10 --- /dev/null +++ b/opensearchpy/metrics/metrics_events.py @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. +# +# Modifications Copyright OpenSearch Contributors. See +# GitHub history for details. + +import time +from typing import Optional + +from events import Events + +from opensearchpy.metrics.metrics import Metrics + + +class MetricsEvents(Metrics): + """ + The MetricsEvents class implements the Metrics abstract base class + and tracks metrics such as start time, end time, and service time + during request processing. + """ + + @property + def start_time(self) -> Optional[float]: + return self._start_time + + @property + def end_time(self) -> Optional[float]: + return self._end_time + + @property + def service_time(self) -> Optional[float]: + return self._service_time + + def __init__(self) -> None: + self.events = Events() + self._start_time: Optional[float] = None + self._end_time: Optional[float] = None + self._service_time: Optional[float] = None + + # Subscribe to the request_start and request_end events + self.events.request_start += self._on_request_start + self.events.request_end += self._on_request_end + + def request_start(self) -> None: + self.events.request_start() + + def _on_request_start(self) -> None: + self._start_time = time.perf_counter() + self._end_time = None + self._service_time = None + + def request_end(self) -> None: + self.events.request_end() + + def _on_request_end(self) -> None: + self._end_time = time.perf_counter() + if self._start_time is not None: + self._service_time = self._end_time - self._start_time diff --git a/opensearchpy/transport.py b/opensearchpy/transport.py index f582a3be..d5492eab 100644 --- a/opensearchpy/transport.py +++ b/opensearchpy/transport.py @@ -29,6 +29,8 @@ from itertools import chain from typing import Any, Callable, Collection, Dict, List, Mapping, Optional, Type, Union +from opensearchpy.metrics.metrics import Metrics + from .connection import Connection, Urllib3HttpConnection from .connection_pool import ConnectionPool, DummyConnectionPool, EmptyConnectionPool from .exceptions import ( @@ -91,6 +93,7 @@ class Transport(object): last_sniff: float sniff_timeout: Optional[float] host_info_callback: Any + metrics: Optional[Metrics] def __init__( self, @@ -112,6 +115,7 @@ def __init__( retry_on_status: Collection[int] = (502, 503, 504), retry_on_timeout: bool = False, send_get_body_as: str = "GET", + metrics: Optional[Metrics] = None, **kwargs: Any ) -> None: """ @@ -153,6 +157,7 @@ def __init__( when creating and instance unless overridden by that connection's options provided as part of the hosts parameter. """ + self.metrics = metrics if connection_class is None: connection_class = self.DEFAULT_CONNECTION_CLASS @@ -242,7 +247,7 @@ def _create_connection(host: Any) -> Any: kwargs.update(host) if self.pool_maxsize and isinstance(self.pool_maxsize, int): kwargs["pool_maxsize"] = self.pool_maxsize - return self.connection_class(**kwargs) + return self.connection_class(metrics=self.metrics, **kwargs) connections = list(zip(map(_create_connection, hosts), hosts)) if len(connections) == 1: diff --git a/setup.py b/setup.py index 057fda18..d9616d83 100644 --- a/setup.py +++ b/setup.py @@ -59,6 +59,7 @@ "six", "python-dateutil", "certifi>=2022.12.07", + "Events", ] tests_require = [ "requests>=2.0.0, <3.0.0", diff --git a/test_opensearchpy/test_server/test_metrics.py b/test_opensearchpy/test_server/test_metrics.py new file mode 100644 index 00000000..036d8c96 --- /dev/null +++ b/test_opensearchpy/test_server/test_metrics.py @@ -0,0 +1,98 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. +# +# Modifications Copyright OpenSearch Contributors. See +# GitHub history for details. + +from __future__ import unicode_literals + +import time + +from opensearchpy import RequestsHttpConnection +from opensearchpy.metrics.metrics_events import MetricsEvents + +from . import OpenSearchTestCase, get_client + + +class TestMetrics(OpenSearchTestCase): + def tearDown(self) -> None: + client = get_client() + client.indices.delete(index=["test-index"], ignore_unavailable=True) + + def test_metrics_default_behavior(self) -> None: + # Test default behavior when metrics is not passed to the client + client = get_client() + index_name = "test-index" + index_body = {"settings": {"index": {"number_of_shards": 4}}} + client.indices.create(index=index_name, body=index_body) + metrics = MetricsEvents() + assert metrics.service_time is None + + +class TestMetricsEvents(OpenSearchTestCase): + def tearDown(self) -> None: + client = get_client() + client.indices.delete(index=["test-index"], ignore_unavailable=True) + + def test_metrics_events_with_urllib3_connection(self) -> None: + # Test MetricsEvents behavior with urllib3 connection + metrics = MetricsEvents() + client = get_client(metrics=metrics) + + # Calculate service time for create index operation + index_name = "test-index" + index_body = {"settings": {"index": {"number_of_shards": 4}}} + start1 = time.perf_counter() + client.indices.create(index=index_name, body=index_body) + duration1 = time.perf_counter() - start1 + create_index_service_time = metrics.service_time + assert ( + isinstance(create_index_service_time, float) + and create_index_service_time < duration1 + ) + + # Calculate service time for adding document operation + document = {"title": "Moneyball", "director": "Bennett Miller", "year": "2011"} + id = "1" + start2 = time.perf_counter() + client.index(index=index_name, body=document, id=id, refresh=True) + duration2 = time.perf_counter() - start2 + assert ( + isinstance(metrics.service_time, float) + and metrics.service_time < duration2 + and metrics.service_time != create_index_service_time + # Above check is to confirm service time differs from the previous API call. + ) + + def test_metrics_events_with_requests_http_connection(self) -> None: + # Test MetricsEvents behavior with requests HTTP connection + metrics = MetricsEvents() + client = get_client(metrics=metrics, connection_class=RequestsHttpConnection) + + # Calculate service time for create index operation + index_name = "test-index" + index_body = {"settings": {"index": {"number_of_shards": 4}}} + start1 = time.perf_counter() + client.indices.create(index_name, body=index_body) + duration1 = time.perf_counter() - start1 + create_index_service_time = metrics.service_time + assert ( + isinstance(create_index_service_time, float) + and create_index_service_time < duration1 + ) + + # Calculate service time for adding document operation + document = {"title": "Moneyball", "director": "Bennett Miller", "year": "2011"} + id = "1" + start2 = time.perf_counter() + client.index(index=index_name, body=document, id=id, refresh=True) + duration2 = time.perf_counter() - start2 + assert ( + isinstance(metrics.service_time, float) + and metrics.service_time < duration2 + and metrics.service_time != create_index_service_time + # Above check is to confirm service time differs from the previous API call. + ) From f963ec8d0d16fe80e13c9cfc4f0138e7e7c2880d Mon Sep 17 00:00:00 2001 From: saimedhi Date: Fri, 12 Apr 2024 11:15:37 -0700 Subject: [PATCH 2/4] Introduced service time metrics to opensearch-py client Signed-off-by: saimedhi --- docs/source/api-ref/metrics.md | 8 +++- opensearchpy/__init__.py | 3 +- opensearchpy/client/client.py | 6 +-- opensearchpy/connection/http_requests.py | 13 ++--- opensearchpy/connection/http_urllib3.py | 13 ++--- opensearchpy/metrics/__init__.py | 2 + opensearchpy/metrics/metrics_none.py | 47 +++++++++++++++++++ opensearchpy/transport.py | 9 ++-- test_opensearchpy/test_server/test_metrics.py | 13 ++++- 9 files changed, 90 insertions(+), 24 deletions(-) create mode 100644 opensearchpy/metrics/metrics_none.py diff --git a/docs/source/api-ref/metrics.md b/docs/source/api-ref/metrics.md index 9c8c6541..dd40d986 100644 --- a/docs/source/api-ref/metrics.md +++ b/docs/source/api-ref/metrics.md @@ -1,10 +1,14 @@ # metrics ```{eval-rst} -.. autoclass:: opensearchpy.metrics.metrics.Metrics +.. autoclass:: opensearchpy.Metrics ``` ```{eval-rst} -.. autoclass:: opensearchpy.metrics.metrics_events.MetricsEvents +.. autoclass:: opensearchpy.MetricsEvents +``` + +```{eval-rst} +.. autoclass:: opensearchpy.MetricsNone ``` diff --git a/opensearchpy/__init__.py b/opensearchpy/__init__.py index f86e18d3..4cf251db 100644 --- a/opensearchpy/__init__.py +++ b/opensearchpy/__init__.py @@ -133,7 +133,7 @@ from .helpers.update_by_query import UpdateByQuery from .helpers.utils import AttrDict, AttrList, DslBase from .helpers.wrappers import Range -from .metrics import Metrics, MetricsEvents +from .metrics import Metrics, MetricsEvents, MetricsNone from .serializer import JSONSerializer from .transport import Transport @@ -243,6 +243,7 @@ "__versionstr__", "Metrics", "MetricsEvents", + "MetricsNone", ] try: diff --git a/opensearchpy/client/client.py b/opensearchpy/client/client.py index adc4b097..091bb5e9 100644 --- a/opensearchpy/client/client.py +++ b/opensearchpy/client/client.py @@ -10,7 +10,6 @@ from typing import Any, Optional, Type from opensearchpy.client.utils import _normalize_hosts -from opensearchpy.metrics.metrics import Metrics from opensearchpy.transport import Transport @@ -23,7 +22,6 @@ def __init__( self, hosts: Optional[str] = None, transport_class: Type[Transport] = Transport, - metrics: Optional[Metrics] = None, **kwargs: Any ) -> None: """ @@ -40,6 +38,4 @@ class as kwargs, or a string in the format of ``host[:port]`` which will be :class:`~opensearchpy.Transport` class and, subsequently, to the :class:`~opensearchpy.Connection` instances. """ - self.transport = transport_class( - _normalize_hosts(hosts), metrics=metrics, **kwargs - ) + self.transport = transport_class(_normalize_hosts(hosts), **kwargs) diff --git a/opensearchpy/connection/http_requests.py b/opensearchpy/connection/http_requests.py index 46518f40..541f034f 100644 --- a/opensearchpy/connection/http_requests.py +++ b/opensearchpy/connection/http_requests.py @@ -36,7 +36,7 @@ except ImportError: REQUESTS_AVAILABLE = False -from opensearchpy.metrics.metrics import Metrics +from opensearchpy.metrics.metrics_none import MetricsNone from ..compat import reraise_exceptions, string_types, urlencode from ..exceptions import ( @@ -71,6 +71,9 @@ class RequestsHttpConnection(Connection): For tracing all requests made by this transport. :arg pool_maxsize: Maximum connection pool size used by pool-manager For custom connection-pooling on current session + :arg metrics: metrics is an instance of a subclass of the + :class:`~opensearchpy.Metrics` class, used for collecting + and reporting metrics related to the client's operations; """ def __init__( @@ -88,7 +91,7 @@ def __init__( http_compress: Any = None, opaque_id: Any = None, pool_maxsize: Any = None, - metrics: Optional[Metrics] = None, + metrics: Any = MetricsNone(), **kwargs: Any ) -> None: self.metrics = metrics @@ -192,8 +195,7 @@ def perform_request( # type: ignore } send_kwargs.update(settings) try: - if self.metrics is not None: - self.metrics.request_start() + self.metrics.request_start() response = self.session.send(prepared_request, **send_kwargs) duration = time.time() - start raw_data = response.content.decode("utf-8", "surrogatepass") @@ -214,8 +216,7 @@ def perform_request( # type: ignore raise ConnectionTimeout("TIMEOUT", str(e), e) raise ConnectionError("N/A", str(e), e) finally: - if self.metrics is not None: - self.metrics.request_end() + self.metrics.request_end() # raise warnings if any from the 'Warnings' header. warnings_headers = ( diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index 21287859..e55fe94f 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -34,7 +34,7 @@ from urllib3.exceptions import SSLError as UrllibSSLError from urllib3.util.retry import Retry -from opensearchpy.metrics.metrics import Metrics +from opensearchpy.metrics.metrics_none import MetricsNone from ..compat import reraise_exceptions, urlencode from ..exceptions import ( @@ -96,6 +96,9 @@ class Urllib3HttpConnection(Connection): :arg http_compress: Use gzip compression :arg opaque_id: Send this value in the 'X-Opaque-Id' HTTP header For tracing all requests made by this transport. + :arg metrics: metrics is an instance of a subclass of the + :class:`~opensearchpy.Metrics` class, used for collecting + and reporting metrics related to the client's operations; """ def __init__( @@ -117,7 +120,7 @@ def __init__( ssl_context: Any = None, http_compress: Any = None, opaque_id: Any = None, - metrics: Optional[Metrics] = None, + metrics: Any = MetricsNone(), **kwargs: Any ) -> None: self.metrics = metrics @@ -272,8 +275,7 @@ def perform_request( if isinstance(self.http_auth, Callable): # type: ignore request_headers.update(self.http_auth(method, full_url, body)) - if self.metrics is not None: - self.metrics.request_start() + self.metrics.request_start() response = self.pool.urlopen( method, url, body, retries=Retry(False), headers=request_headers, **kw @@ -292,8 +294,7 @@ def perform_request( raise ConnectionTimeout("TIMEOUT", str(e), e) raise ConnectionError("N/A", str(e), e) finally: - if self.metrics is not None: - self.metrics.request_end() + self.metrics.request_end() # raise warnings if any from the 'Warnings' header. warning_headers = response.headers.get_all("warning", ()) diff --git a/opensearchpy/metrics/__init__.py b/opensearchpy/metrics/__init__.py index 38946518..2d0e0d11 100644 --- a/opensearchpy/metrics/__init__.py +++ b/opensearchpy/metrics/__init__.py @@ -9,8 +9,10 @@ from .metrics import Metrics from .metrics_events import MetricsEvents +from .metrics_none import MetricsNone __all__ = [ "Metrics", "MetricsEvents", + "MetricsNone", ] diff --git a/opensearchpy/metrics/metrics_none.py b/opensearchpy/metrics/metrics_none.py new file mode 100644 index 00000000..bbc7b335 --- /dev/null +++ b/opensearchpy/metrics/metrics_none.py @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. +# +# Modifications Copyright OpenSearch Contributors. See +# GitHub history for details. + +from typing import Optional + +from opensearchpy.metrics.metrics import Metrics + + +class MetricsNone(Metrics): + """ + Default metrics class. It sets the start time, end time, and service time to None. + """ + + @property + def start_time(self) -> Optional[float]: + return self._start_time + + @property + def end_time(self) -> Optional[float]: + return self._end_time + + @property + def service_time(self) -> Optional[float]: + return self._service_time + + def __init__(self) -> None: + self._start_time: Optional[float] = None + self._end_time: Optional[float] = None + self._service_time: Optional[float] = None + + # request_start and request_end are placeholders, + # not implementing actual metrics collection in this subclass. + + def request_start(self) -> None: + self._start_time = None + self._end_time = None + self._service_time = None + + def request_end(self) -> None: + self._end_time = None + self._service_time = None diff --git a/opensearchpy/transport.py b/opensearchpy/transport.py index d5492eab..4af1fee0 100644 --- a/opensearchpy/transport.py +++ b/opensearchpy/transport.py @@ -29,7 +29,7 @@ from itertools import chain from typing import Any, Callable, Collection, Dict, List, Mapping, Optional, Type, Union -from opensearchpy.metrics.metrics import Metrics +from opensearchpy.metrics.metrics_none import MetricsNone from .connection import Connection, Urllib3HttpConnection from .connection_pool import ConnectionPool, DummyConnectionPool, EmptyConnectionPool @@ -93,7 +93,7 @@ class Transport(object): last_sniff: float sniff_timeout: Optional[float] host_info_callback: Any - metrics: Optional[Metrics] + metrics: Any def __init__( self, @@ -115,7 +115,7 @@ def __init__( retry_on_status: Collection[int] = (502, 503, 504), retry_on_timeout: bool = False, send_get_body_as: str = "GET", - metrics: Optional[Metrics] = None, + metrics: Any = MetricsNone(), **kwargs: Any ) -> None: """ @@ -152,6 +152,9 @@ def __init__( will be serialized and passed as a query parameter `source`. :arg pool_maxsize: Maximum connection pool size used by pool-manager For custom connection-pooling on current session + :arg metrics: metrics is an instance of a subclass of the + :class:`~opensearchpy.Metrics` class, used for collecting + and reporting metrics related to the client's operations; Any extra keyword arguments will be passed to the `connection_class` when creating and instance unless overridden by that connection's diff --git a/test_opensearchpy/test_server/test_metrics.py b/test_opensearchpy/test_server/test_metrics.py index 036d8c96..df201ef6 100644 --- a/test_opensearchpy/test_server/test_metrics.py +++ b/test_opensearchpy/test_server/test_metrics.py @@ -13,6 +13,7 @@ from opensearchpy import RequestsHttpConnection from opensearchpy.metrics.metrics_events import MetricsEvents +from opensearchpy.metrics.metrics_none import MetricsNone from . import OpenSearchTestCase, get_client @@ -27,8 +28,18 @@ def test_metrics_default_behavior(self) -> None: client = get_client() index_name = "test-index" index_body = {"settings": {"index": {"number_of_shards": 4}}} + try: + client.indices.create(index=index_name, body=index_body) + except Exception as e: + assert False, f"Error creating index: {e}" + + def test_metrics_none_behavior(self) -> None: + # Test behavior when metrics is an instance of MetricsNone + metrics = MetricsNone() + client = get_client(metrics=metrics) + index_name = "test-index" + index_body = {"settings": {"index": {"number_of_shards": 4}}} client.indices.create(index=index_name, body=index_body) - metrics = MetricsEvents() assert metrics.service_time is None From 9e147b210eccf80d3de041929ce54cc2cf090463 Mon Sep 17 00:00:00 2001 From: saimedhi Date: Fri, 12 Apr 2024 11:16:59 -0700 Subject: [PATCH 3/4] Introduced service time metrics to opensearch-py client Signed-off-by: saimedhi --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4b3dd1..97cfc03c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] ### Added - Added support for Python 3.12 ([#717](https://github.com/opensearch-project/opensearch-py/pull/717)) +- Introduced `service time` metrics to opensearch-py client ([#716](https://github.com/opensearch-project/opensearch-py/pull/716)) ### Changed ### Deprecated ### Removed @@ -30,7 +31,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Enhance generator to generate plugins ([#700](https://github.com/opensearch-project/opensearch-py/pull/700)) - Enhance generator to update changelog only if generated code differs from existing ([#684](https://github.com/opensearch-project/opensearch-py/pull/684)) - Added guide for configuring ssl_assert_hostname ([#694](https://github.com/opensearch-project/opensearch-py/pull/694)) -- Introduced `service time` metrics to opensearch-py client ([#716](https://github.com/opensearch-project/opensearch-py/pull/716)) ### Changed - Updated the `get_policy` API in the index_management plugin to allow the policy_id argument as optional ([#633](https://github.com/opensearch-project/opensearch-py/pull/633)) - Updated the `point_in_time.md` guide with examples demonstrating the usage of the new APIs as alternatives to the deprecated ones. ([#661](https://github.com/opensearch-project/opensearch-py/pull/661)) From a625715cb8f5c8054af7d3dde5e7b14409f5af83 Mon Sep 17 00:00:00 2001 From: saimedhi Date: Sun, 14 Apr 2024 14:57:33 -0700 Subject: [PATCH 4/4] Added service time metrics Signed-off-by: saimedhi --- CHANGELOG.md | 2 +- opensearchpy/connection/http_requests.py | 4 ++-- opensearchpy/connection/http_urllib3.py | 4 ++-- opensearchpy/transport.py | 6 +++--- test_opensearchpy/test_server/test_metrics.py | 8 ++++++++ 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97cfc03c..06fd1386 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] ### Added - Added support for Python 3.12 ([#717](https://github.com/opensearch-project/opensearch-py/pull/717)) -- Introduced `service time` metrics to opensearch-py client ([#716](https://github.com/opensearch-project/opensearch-py/pull/716)) +- Added service time metrics ([#716](https://github.com/opensearch-project/opensearch-py/pull/716)) ### Changed ### Deprecated ### Removed diff --git a/opensearchpy/connection/http_requests.py b/opensearchpy/connection/http_requests.py index 541f034f..2c173725 100644 --- a/opensearchpy/connection/http_requests.py +++ b/opensearchpy/connection/http_requests.py @@ -36,7 +36,7 @@ except ImportError: REQUESTS_AVAILABLE = False -from opensearchpy.metrics.metrics_none import MetricsNone +from opensearchpy.metrics import Metrics, MetricsNone from ..compat import reraise_exceptions, string_types, urlencode from ..exceptions import ( @@ -91,7 +91,7 @@ def __init__( http_compress: Any = None, opaque_id: Any = None, pool_maxsize: Any = None, - metrics: Any = MetricsNone(), + metrics: Metrics = MetricsNone(), **kwargs: Any ) -> None: self.metrics = metrics diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index e55fe94f..e3b60cf3 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -34,7 +34,7 @@ from urllib3.exceptions import SSLError as UrllibSSLError from urllib3.util.retry import Retry -from opensearchpy.metrics.metrics_none import MetricsNone +from opensearchpy.metrics import Metrics, MetricsNone from ..compat import reraise_exceptions, urlencode from ..exceptions import ( @@ -120,7 +120,7 @@ def __init__( ssl_context: Any = None, http_compress: Any = None, opaque_id: Any = None, - metrics: Any = MetricsNone(), + metrics: Metrics = MetricsNone(), **kwargs: Any ) -> None: self.metrics = metrics diff --git a/opensearchpy/transport.py b/opensearchpy/transport.py index 4af1fee0..5c7e6297 100644 --- a/opensearchpy/transport.py +++ b/opensearchpy/transport.py @@ -29,7 +29,7 @@ from itertools import chain from typing import Any, Callable, Collection, Dict, List, Mapping, Optional, Type, Union -from opensearchpy.metrics.metrics_none import MetricsNone +from opensearchpy.metrics import Metrics, MetricsNone from .connection import Connection, Urllib3HttpConnection from .connection_pool import ConnectionPool, DummyConnectionPool, EmptyConnectionPool @@ -93,7 +93,7 @@ class Transport(object): last_sniff: float sniff_timeout: Optional[float] host_info_callback: Any - metrics: Any + metrics: Metrics def __init__( self, @@ -115,7 +115,7 @@ def __init__( retry_on_status: Collection[int] = (502, 503, 504), retry_on_timeout: bool = False, send_get_body_as: str = "GET", - metrics: Any = MetricsNone(), + metrics: Metrics = MetricsNone(), **kwargs: Any ) -> None: """ diff --git a/test_opensearchpy/test_server/test_metrics.py b/test_opensearchpy/test_server/test_metrics.py index df201ef6..189fc739 100644 --- a/test_opensearchpy/test_server/test_metrics.py +++ b/test_opensearchpy/test_server/test_metrics.py @@ -11,6 +11,8 @@ import time +import pytest + from opensearchpy import RequestsHttpConnection from opensearchpy.metrics.metrics_events import MetricsEvents from opensearchpy.metrics.metrics_none import MetricsNone @@ -33,6 +35,12 @@ def test_metrics_default_behavior(self) -> None: except Exception as e: assert False, f"Error creating index: {e}" + def test_metrics_raises_error_when_value_is_none(self) -> None: + # Test behavior when metrics is given None. + metrics = None + with pytest.raises(AttributeError): + get_client(metrics=metrics) + def test_metrics_none_behavior(self) -> None: # Test behavior when metrics is an instance of MetricsNone metrics = MetricsNone()