From f5300b3323364dbf6ba845ecb6058c56819e85e2 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Thu, 7 Mar 2024 19:35:14 +0800 Subject: [PATCH 01/11] Remove thread lock by loading RuntimeContext explicitly. --- .../src/opentelemetry/context/__init__.py | 72 +++++++------------ 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index 91133884f68..ed380355085 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -25,55 +25,38 @@ from opentelemetry.util._importlib_metadata import entry_points logger = logging.getLogger(__name__) -_RUNTIME_CONTEXT = None # type: typing.Optional[_RuntimeContext] -_RUNTIME_CONTEXT_LOCK = threading.Lock() -_F = typing.TypeVar("_F", bound=typing.Callable[..., typing.Any]) - - -def _load_runtime_context(func: _F) -> _F: - """A decorator used to initialize the global RuntimeContext +def _load_runtime_context() -> typing.Optional[_RuntimeContext]: + """Initialize the RuntimeContext Returns: - A wrapper of the decorated method. + An instance of RuntimeContext. """ - @wraps(func) # type: ignore[misc] - def wrapper( - *args: typing.Tuple[typing.Any, typing.Any], - **kwargs: typing.Dict[typing.Any, typing.Any], - ) -> typing.Optional[typing.Any]: - global _RUNTIME_CONTEXT # pylint: disable=global-statement - - with _RUNTIME_CONTEXT_LOCK: - if _RUNTIME_CONTEXT is None: - # FIXME use a better implementation of a configuration manager - # to avoid having to get configuration values straight from - # environment variables - default_context = "contextvars_context" - - configured_context = environ.get( - OTEL_PYTHON_CONTEXT, default_context - ) # type: str - try: - - _RUNTIME_CONTEXT = next( # type: ignore - iter( # type: ignore - entry_points( # type: ignore - group="opentelemetry_context", - name=configured_context, - ) - ) - ).load()() - - except Exception: # pylint: disable=broad-except - logger.exception( - "Failed to load context: %s", configured_context - ) - return func(*args, **kwargs) # type: ignore[misc] - - return typing.cast(_F, wrapper) # type: ignore[misc] + # FIXME use a better implementation of a configuration manager + # to avoid having to get configuration values straight from + # environment variables + default_context = "contextvars_context" + + configured_context = environ.get( + OTEL_PYTHON_CONTEXT, default_context + ) # type: str + + try: + return next( # type: ignore + iter( # type: ignore + entry_points( # type: ignore + group="opentelemetry_context", + name=configured_context, + ) + ) + ).load()() + except Exception: # pylint: disable=broad-except + logger.exception( + "Failed to load context: %s", configured_context + ) +_RUNTIME_CONTEXT = _load_runtime_context() def create_key(keyname: str) -> str: """To allow cross-cutting concern to control access to their local state, @@ -125,7 +108,6 @@ def set_value( return Context(new_values) -@_load_runtime_context # type: ignore def get_current() -> Context: """To access the context associated with program execution, the Context API provides a function which takes no arguments @@ -137,7 +119,6 @@ def get_current() -> Context: return _RUNTIME_CONTEXT.get_current() # type:ignore -@_load_runtime_context # type: ignore def attach(context: Context) -> object: """Associates a Context with the caller's current execution unit. Returns a token that can be used to restore the previous Context. @@ -151,7 +132,6 @@ def attach(context: Context) -> object: return _RUNTIME_CONTEXT.attach(context) # type:ignore -@_load_runtime_context # type: ignore def detach(token: object) -> None: """Resets the Context associated with the caller's current execution unit to the value it had before attaching a specified Context. From 1f6f90eb7670702557d187e8832d14b9bf5dfc22 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Mon, 25 Mar 2024 16:22:25 +0800 Subject: [PATCH 02/11] Add test for _load_runtime_context --- opentelemetry-api/tests/context/test_context.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/opentelemetry-api/tests/context/test_context.py b/opentelemetry-api/tests/context/test_context.py index b90c011b992..3f3fc50badf 100644 --- a/opentelemetry-api/tests/context/test_context.py +++ b/opentelemetry-api/tests/context/test_context.py @@ -74,3 +74,17 @@ def test_set_current(self): context.detach(token) self.assertEqual("yyy", context.get_value("a")) + +class TestInitContext(unittest.TestCase): + def test_load_runtime_context(self): + from os import environ + from opentelemetry.environment_variables import OTEL_PYTHON_CONTEXT + from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext + + environ[OTEL_PYTHON_CONTEXT] = "contextvars_context" + ctx = context._load_runtime_context() + self.assertIsInstance(ctx, ContextVarsRuntimeContext) + + environ.pop(OTEL_PYTHON_CONTEXT) + ctx = context._load_runtime_context() + self.assertIsInstance(ctx, ContextVarsRuntimeContext) From 703ad6253fa67070c01169eb5b52ca864fd48ad2 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Wed, 3 Apr 2024 14:53:51 +0800 Subject: [PATCH 03/11] Fix mypy check by make _load_runtime_context failfast --- .../src/opentelemetry/context/__init__.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index ed380355085..d0fb4b89151 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -42,19 +42,14 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: OTEL_PYTHON_CONTEXT, default_context ) # type: str - try: - return next( # type: ignore - iter( # type: ignore - entry_points( # type: ignore - group="opentelemetry_context", - name=configured_context, - ) + return next( # type: ignore + iter( # type: ignore + entry_points( # type: ignore + group="opentelemetry_context", + name=configured_context, ) - ).load()() - except Exception: # pylint: disable=broad-except - logger.exception( - "Failed to load context: %s", configured_context ) + ).load()() _RUNTIME_CONTEXT = _load_runtime_context() From e6712f3d17e550f5f0f6b9e50e3ab4a02efb08f2 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Fri, 12 Apr 2024 00:05:37 +0800 Subject: [PATCH 04/11] Fix lint --- .../src/opentelemetry/context/__init__.py | 5 +++-- opentelemetry-api/tests/context/test_context.py | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index d0fb4b89151..031be8d4726 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -13,9 +13,7 @@ # limitations under the License. import logging -import threading import typing -from functools import wraps from os import environ from uuid import uuid4 @@ -26,6 +24,7 @@ logger = logging.getLogger(__name__) + def _load_runtime_context() -> typing.Optional[_RuntimeContext]: """Initialize the RuntimeContext @@ -51,8 +50,10 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: ) ).load()() + _RUNTIME_CONTEXT = _load_runtime_context() + def create_key(keyname: str) -> str: """To allow cross-cutting concern to control access to their local state, the RuntimeContext API provides a function which takes a keyname as input, diff --git a/opentelemetry-api/tests/context/test_context.py b/opentelemetry-api/tests/context/test_context.py index 3f3fc50badf..6291e8e50d1 100644 --- a/opentelemetry-api/tests/context/test_context.py +++ b/opentelemetry-api/tests/context/test_context.py @@ -13,9 +13,12 @@ # limitations under the License. import unittest +from os import environ from opentelemetry import context from opentelemetry.context.context import Context +from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext +from opentelemetry.environment_variables import OTEL_PYTHON_CONTEXT def _do_work() -> str: @@ -75,16 +78,13 @@ def test_set_current(self): context.detach(token) self.assertEqual("yyy", context.get_value("a")) + class TestInitContext(unittest.TestCase): def test_load_runtime_context(self): - from os import environ - from opentelemetry.environment_variables import OTEL_PYTHON_CONTEXT - from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext - environ[OTEL_PYTHON_CONTEXT] = "contextvars_context" - ctx = context._load_runtime_context() + ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) environ.pop(OTEL_PYTHON_CONTEXT) - ctx = context._load_runtime_context() + ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) From 80404b01566cfffe5e20d352ac5079e9b6bf46d1 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Fri, 12 Apr 2024 21:26:53 +0800 Subject: [PATCH 05/11] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23077ff4b64..dd9b0b19edb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Remove thread lock by loading RuntimeContext explicitly. + ([#3763](https://github.com/open-telemetry/opentelemetry-python/pull/3763)) - Update proto version to v1.2.0 ([#3844](https://github.com/open-telemetry/opentelemetry-python/pull/3844)) - Add to_json method to ExponentialHistogram From cc6d5da6ac7b6cee041db8c3d52cdbc2cc967608 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Thu, 18 Apr 2024 11:22:45 +0800 Subject: [PATCH 06/11] Restore exception handling --- .../src/opentelemetry/context/__init__.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index 031be8d4726..887d6930c54 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -41,14 +41,18 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: OTEL_PYTHON_CONTEXT, default_context ) # type: str - return next( # type: ignore - iter( # type: ignore - entry_points( # type: ignore - group="opentelemetry_context", - name=configured_context, + try: + return next( # type: ignore + iter( # type: ignore + entry_points( # type: ignore + group="opentelemetry_context", + name=configured_context, + ) ) - ) - ).load()() + ).load()() + except Exception: # pylint: disable=broad-except + logger.exception("Failed to load context: %s", configured_context) + return None _RUNTIME_CONTEXT = _load_runtime_context() From ca19dc275b6faf9d8ca5e13cc6b82cecdcf25e0b Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Mon, 22 Apr 2024 19:31:33 +0800 Subject: [PATCH 07/11] Resolve conversation --- .../src/opentelemetry/context/__init__.py | 15 +++++++++++++-- opentelemetry-api/tests/context/test_context.py | 8 ++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index 887d6930c54..da92d1f51b5 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -51,8 +51,19 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: ) ).load()() except Exception: # pylint: disable=broad-except - logger.exception("Failed to load context: %s", configured_context) - return None + logger.exception( + "Failed to load context: %s, fallback to %s", + configured_context, + OTEL_PYTHON_CONTEXT, + ) + return next( # type: ignore + iter( # type: ignore + entry_points( # type: ignore + group="opentelemetry_context", + name=OTEL_PYTHON_CONTEXT, + ) + ) + ).load()() _RUNTIME_CONTEXT = _load_runtime_context() diff --git a/opentelemetry-api/tests/context/test_context.py b/opentelemetry-api/tests/context/test_context.py index 6291e8e50d1..4a1e854e830 100644 --- a/opentelemetry-api/tests/context/test_context.py +++ b/opentelemetry-api/tests/context/test_context.py @@ -13,7 +13,7 @@ # limitations under the License. import unittest -from os import environ +from unittest.mock import patch from opentelemetry import context from opentelemetry.context.context import Context @@ -80,11 +80,11 @@ def test_set_current(self): class TestInitContext(unittest.TestCase): - def test_load_runtime_context(self): - environ[OTEL_PYTHON_CONTEXT] = "contextvars_context" + def test_load_runtime_context_default(self): ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) - environ.pop(OTEL_PYTHON_CONTEXT) + @patch.dict("os.environ", {OTEL_PYTHON_CONTEXT: "contextvars_context"}) + def test_load_runtime_context(self): ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) From 263e3c04e25bda61b0f0113b4b9c3fafd9a2536d Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Tue, 23 Apr 2024 17:47:01 +0800 Subject: [PATCH 08/11] Add test_load_runtime_context_fallback --- opentelemetry-api/src/opentelemetry/context/__init__.py | 2 +- opentelemetry-api/tests/context/test_context.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index da92d1f51b5..e6ea625b42e 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -60,7 +60,7 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: iter( # type: ignore entry_points( # type: ignore group="opentelemetry_context", - name=OTEL_PYTHON_CONTEXT, + name=default_context, ) ) ).load()() diff --git a/opentelemetry-api/tests/context/test_context.py b/opentelemetry-api/tests/context/test_context.py index 4a1e854e830..af5b38ef88f 100644 --- a/opentelemetry-api/tests/context/test_context.py +++ b/opentelemetry-api/tests/context/test_context.py @@ -88,3 +88,8 @@ def test_load_runtime_context_default(self): def test_load_runtime_context(self): ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) + + @patch.dict("os.environ", {OTEL_PYTHON_CONTEXT: "foo"}) + def test_load_runtime_context_fallback(self): + ctx = context._load_runtime_context() # pylint: disable=W0212 + self.assertIsInstance(ctx, ContextVarsRuntimeContext) From ea17a3e4ec02151abfa1447ced83797b221c23a9 Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Wed, 24 Apr 2024 17:24:03 +0800 Subject: [PATCH 09/11] Fix --- opentelemetry-api/src/opentelemetry/context/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index e6ea625b42e..800020454f3 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -54,7 +54,7 @@ def _load_runtime_context() -> typing.Optional[_RuntimeContext]: logger.exception( "Failed to load context: %s, fallback to %s", configured_context, - OTEL_PYTHON_CONTEXT, + default_context, ) return next( # type: ignore iter( # type: ignore From 5bff39b1b272a02dc38e756564504315835ede9c Mon Sep 17 00:00:00 2001 From: WqyJh <781345688@qq.com> Date: Thu, 25 Apr 2024 14:11:19 +0800 Subject: [PATCH 10/11] Fix type hint --- opentelemetry-api/src/opentelemetry/context/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/__init__.py b/opentelemetry-api/src/opentelemetry/context/__init__.py index 800020454f3..0a2785ab607 100644 --- a/opentelemetry-api/src/opentelemetry/context/__init__.py +++ b/opentelemetry-api/src/opentelemetry/context/__init__.py @@ -25,7 +25,7 @@ logger = logging.getLogger(__name__) -def _load_runtime_context() -> typing.Optional[_RuntimeContext]: +def _load_runtime_context() -> _RuntimeContext: """Initialize the RuntimeContext Returns: @@ -127,7 +127,7 @@ def get_current() -> Context: Returns: The current `Context` object. """ - return _RUNTIME_CONTEXT.get_current() # type:ignore + return _RUNTIME_CONTEXT.get_current() def attach(context: Context) -> object: @@ -140,7 +140,7 @@ def attach(context: Context) -> object: Returns: A token that can be used with `detach` to reset the context. """ - return _RUNTIME_CONTEXT.attach(context) # type:ignore + return _RUNTIME_CONTEXT.attach(context) def detach(token: object) -> None: @@ -151,7 +151,7 @@ def detach(token: object) -> None: token: The Token that was returned by a previous call to attach a Context. """ try: - _RUNTIME_CONTEXT.detach(token) # type: ignore + _RUNTIME_CONTEXT.detach(token) except Exception: # pylint: disable=broad-except logger.exception("Failed to detach context") From 959698e9fc8ca3b42d077827f99c223ecafe4ee3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 2 May 2024 10:17:14 -0700 Subject: [PATCH 11/11] Update test_context.py --- opentelemetry-api/tests/context/test_context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/tests/context/test_context.py b/opentelemetry-api/tests/context/test_context.py index af5b38ef88f..18f6f68a514 100644 --- a/opentelemetry-api/tests/context/test_context.py +++ b/opentelemetry-api/tests/context/test_context.py @@ -85,11 +85,11 @@ def test_load_runtime_context_default(self): self.assertIsInstance(ctx, ContextVarsRuntimeContext) @patch.dict("os.environ", {OTEL_PYTHON_CONTEXT: "contextvars_context"}) - def test_load_runtime_context(self): + def test_load_runtime_context(self): # type: ignore[misc] ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext) @patch.dict("os.environ", {OTEL_PYTHON_CONTEXT: "foo"}) - def test_load_runtime_context_fallback(self): + def test_load_runtime_context_fallback(self): # type: ignore[misc] ctx = context._load_runtime_context() # pylint: disable=W0212 self.assertIsInstance(ctx, ContextVarsRuntimeContext)