From 04e1f9831e882ea4f5a1600f74cfe75e94e91687 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 12:04:22 +0530 Subject: [PATCH 01/12] Add traced_request_attrs definition to config --- .../src/opentelemetry/configuration/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index bf641ca836d..686832df512 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -95,7 +95,7 @@ from os import environ from re import fullmatch -from typing import ClassVar, Dict, Optional, TypeVar, Union +from typing import ClassVar, Dict, Optional, TypeVar, Union, List ConfigValue = Union[str, bool, int, float] _T = TypeVar("_T", ConfigValue, Optional[ConfigValue]) @@ -167,3 +167,11 @@ def _reset(cls) -> None: if cls._instance: cls._instance._config_map.clear() # pylint: disable=protected-access cls._instance = None + + def traced_request_attrs(self, instrumentation: str) -> List[str]: + key = "{}_TRACED_REQUEST_ATTRS".format(instrumentation.upper()) + value = self.get(key, "") + request_attrs = ( + [attr.strip() for attr in str.split(value, ",")] if value else [] + ) + return request_attrs From c2e60b58563941834fbd223e7f1881688aa24845 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 12:04:51 +0530 Subject: [PATCH 02/12] Add tests for traced_request_attrs --- .../tests/configuration/test_configuration.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index 608a96c96d1..54b4df67e8f 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -145,3 +145,17 @@ def test_float(self) -> None: self.assertEqual( Configuration().NON_FLOAT, "-12z3.123" ) # pylint: disable=no-member + + @patch.dict( + "os.environ", # type: ignore + { + "OTEL_PYTHON_WEBFRAMEWORK_TRACED_REQUEST_ATTRS": "content_type,keep_alive", + }, + ) + def test_traced_request_attrs(self) -> None: + cfg = Configuration() + request_attrs = cfg.traced_request_attrs("webframework") + self.assertEqual(len(request_attrs), 2) + self.assertIn("content_type", request_attrs) + self.assertIn("keep_alive", request_attrs) + self.assertNotIn("authorization", request_attrs) From 6ff672d7684ce2cfa16c65d7544fc1bc3fce89e5 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 13:03:54 +0530 Subject: [PATCH 03/12] Add excluded_urls definition for config --- .../opentelemetry/configuration/__init__.py | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 686832df512..9727cddf7af 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -93,14 +93,26 @@ to override this value instead of changing it. """ +import re from os import environ -from re import fullmatch -from typing import ClassVar, Dict, Optional, TypeVar, Union, List +from typing import ClassVar, Dict, List, Optional, Sequence, TypeVar, Union ConfigValue = Union[str, bool, int, float] _T = TypeVar("_T", ConfigValue, Optional[ConfigValue]) +class ExcludeList: + """Class to exclude certain paths (given as a list of regexes) from tracing requests""" + + def __init__(self, excluded_urls: Sequence[str]): + self._non_empty = len(excluded_urls) > 0 + if self._non_empty: + self._regex = re.compile("|".join(excluded_urls)) + + def url_disabled(self, url: str) -> bool: + return bool(self._non_empty and re.search(self._regex, url)) + + class Configuration: _instance = None # type: ClassVar[Optional[Configuration]] _config_map = {} # type: ClassVar[Dict[str, ConfigValue]] @@ -113,7 +125,7 @@ def __new__(cls) -> "Configuration": instance = super().__new__(cls) for key, value_str in environ.items(): - match = fullmatch(r"OTEL_(PYTHON_)?([A-Za-z_][\w_]*)", key) + match = re.fullmatch(r"OTEL_(PYTHON_)?([A-Za-z_][\w_]*)", key) if match is not None: @@ -175,3 +187,9 @@ def traced_request_attrs(self, instrumentation: str) -> List[str]: [attr.strip() for attr in str.split(value, ",")] if value else [] ) return request_attrs + + def excluded_urls(self, instrumentation: str) -> ExcludeList: + key = "{}_EXCLUDED_URLS".format(instrumentation.upper()) + value = self.get(key, "") + urls = str.split(value, ",") if value else [] + return ExcludeList(urls) From da0bcef3050d84e01119f44fe6ae6133004e0f92 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 13:07:11 +0530 Subject: [PATCH 04/12] Move ExcludeList from util to config --- opentelemetry-api/src/opentelemetry/util/__init__.py | 12 ------------ .../{util => configuration}/test_exclude_list.py | 3 +-- 2 files changed, 1 insertion(+), 14 deletions(-) rename opentelemetry-api/tests/{util => configuration}/test_exclude_list.py (97%) diff --git a/opentelemetry-api/src/opentelemetry/util/__init__.py b/opentelemetry-api/src/opentelemetry/util/__init__.py index 5d98ba96bf6..58c297269e0 100644 --- a/opentelemetry-api/src/opentelemetry/util/__init__.py +++ b/opentelemetry-api/src/opentelemetry/util/__init__.py @@ -65,15 +65,3 @@ def _load_meter_provider(provider: str) -> "MeterProvider": def _load_trace_provider(provider: str) -> "TracerProvider": return cast("TracerProvider", _load_provider(provider)) - - -class ExcludeList: - """Class to exclude certain paths (given as a list of regexes) from tracing requests""" - - def __init__(self, excluded_urls: Sequence[str]): - self._non_empty = len(excluded_urls) > 0 - if self._non_empty: - self._regex = re.compile("|".join(excluded_urls)) - - def url_disabled(self, url: str) -> bool: - return bool(self._non_empty and re.search(self._regex, url)) diff --git a/opentelemetry-api/tests/util/test_exclude_list.py b/opentelemetry-api/tests/configuration/test_exclude_list.py similarity index 97% rename from opentelemetry-api/tests/util/test_exclude_list.py rename to opentelemetry-api/tests/configuration/test_exclude_list.py index da51478de3e..c7baf842ed7 100644 --- a/opentelemetry-api/tests/util/test_exclude_list.py +++ b/opentelemetry-api/tests/configuration/test_exclude_list.py @@ -13,9 +13,8 @@ # limitations under the License. import unittest -from unittest import mock -from opentelemetry.util import ExcludeList +from opentelemetry.configuration import ExcludeList class TestExcludeList(unittest.TestCase): From be19c3a02e6d0c3d66031a7a25a0bc03d6707384 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 13:07:36 +0530 Subject: [PATCH 05/12] Add tests for excluded_urls --- .../tests/configuration/test_configuration.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index 54b4df67e8f..74c88d141c0 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -159,3 +159,18 @@ def test_traced_request_attrs(self) -> None: self.assertIn("content_type", request_attrs) self.assertIn("keep_alive", request_attrs) self.assertNotIn("authorization", request_attrs) + + @patch.dict( + "os.environ", # type: ignore + { + "OTEL_PYTHON_WEBFRAMEWORK_EXCLUDED_URLS": "/healthzz,path,/issues/.*/view", + }, + ) + def test_excluded_urls(self) -> None: + cfg = Configuration() + excluded_urls = cfg.excluded_urls("webframework") + self.assertTrue(excluded_urls.url_disabled("/healthzz")) + self.assertTrue(excluded_urls.url_disabled("/path")) + self.assertTrue(excluded_urls.url_disabled("/issues/123/view")) + self.assertFalse(excluded_urls.url_disabled("/issues")) + self.assertFalse(excluded_urls.url_disabled("/hello")) From 6870b0139b669d88fe055b64eade067ca814d519 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 21:10:17 +0530 Subject: [PATCH 06/12] Update configuration --- .../src/opentelemetry/configuration/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 9727cddf7af..7c85a61197e 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -181,8 +181,10 @@ def _reset(cls) -> None: cls._instance = None def traced_request_attrs(self, instrumentation: str) -> List[str]: + """Returns list of traced request attributes for instrumentation.""" key = "{}_TRACED_REQUEST_ATTRS".format(instrumentation.upper()) - value = self.get(key, "") + value = self._config_map.get(key, "") + request_attrs = ( [attr.strip() for attr in str.split(value, ",")] if value else [] ) @@ -190,6 +192,7 @@ def traced_request_attrs(self, instrumentation: str) -> List[str]: def excluded_urls(self, instrumentation: str) -> ExcludeList: key = "{}_EXCLUDED_URLS".format(instrumentation.upper()) - value = self.get(key, "") + value = self._config_map.get(key, "") + urls = str.split(value, ",") if value else [] return ExcludeList(urls) From d5003849ce69affc42e84476567fabb4476d3699 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 26 Nov 2020 21:25:23 +0530 Subject: [PATCH 07/12] Type check --- opentelemetry-api/src/opentelemetry/configuration/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 7c85a61197e..d1325d92691 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -186,7 +186,7 @@ def traced_request_attrs(self, instrumentation: str) -> List[str]: value = self._config_map.get(key, "") request_attrs = ( - [attr.strip() for attr in str.split(value, ",")] if value else [] + [attr.strip() for attr in str.split(value, ",")] if value else [] # type: ignore ) return request_attrs @@ -194,5 +194,5 @@ def excluded_urls(self, instrumentation: str) -> ExcludeList: key = "{}_EXCLUDED_URLS".format(instrumentation.upper()) value = self._config_map.get(key, "") - urls = str.split(value, ",") if value else [] + urls = str.split(value, ",") if value else [] # type: ignore return ExcludeList(urls) From 64e5188bb48a2d1c3a0bfd703edf2c0b86f64339 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 27 Nov 2020 00:45:31 +0530 Subject: [PATCH 08/12] Update contrib repo SHA --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ce3c100d0ae..b26fb331bcb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: bcec49cf2eccf8da66c9e63b9836ea8a20516efc + CONTRIB_REPO_SHA: 61ae201677be80e333746f06bc9485fa8856f42c jobs: build: From ce6449accf315977dd1eab940f7f49b7d894618f Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 4 Dec 2020 00:51:11 +0530 Subject: [PATCH 09/12] Make methods internal --- opentelemetry-api/src/opentelemetry/configuration/__init__.py | 4 ++-- opentelemetry-api/tests/configuration/test_configuration.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index d1325d92691..5e3d3667dc1 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -180,7 +180,7 @@ def _reset(cls) -> None: cls._instance._config_map.clear() # pylint: disable=protected-access cls._instance = None - def traced_request_attrs(self, instrumentation: str) -> List[str]: + def _traced_request_attrs(self, instrumentation: str) -> List[str]: """Returns list of traced request attributes for instrumentation.""" key = "{}_TRACED_REQUEST_ATTRS".format(instrumentation.upper()) value = self._config_map.get(key, "") @@ -190,7 +190,7 @@ def traced_request_attrs(self, instrumentation: str) -> List[str]: ) return request_attrs - def excluded_urls(self, instrumentation: str) -> ExcludeList: + def _excluded_urls(self, instrumentation: str) -> ExcludeList: key = "{}_EXCLUDED_URLS".format(instrumentation.upper()) value = self._config_map.get(key, "") diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index 74c88d141c0..de7bb16de6e 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -154,7 +154,7 @@ def test_float(self) -> None: ) def test_traced_request_attrs(self) -> None: cfg = Configuration() - request_attrs = cfg.traced_request_attrs("webframework") + request_attrs = cfg._traced_request_attrs("webframework") self.assertEqual(len(request_attrs), 2) self.assertIn("content_type", request_attrs) self.assertIn("keep_alive", request_attrs) @@ -168,7 +168,7 @@ def test_traced_request_attrs(self) -> None: ) def test_excluded_urls(self) -> None: cfg = Configuration() - excluded_urls = cfg.excluded_urls("webframework") + excluded_urls = cfg._excluded_urls("webframework") self.assertTrue(excluded_urls.url_disabled("/healthzz")) self.assertTrue(excluded_urls.url_disabled("/path")) self.assertTrue(excluded_urls.url_disabled("/issues/123/view")) From aaff2f4671a5ce622730672af913e825183c0852 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 4 Dec 2020 01:01:57 +0530 Subject: [PATCH 10/12] Update contrib repo SHA --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b1c8c95f753..e71b58f94ae 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: 5a763dcdf38732b3aaf0e940e7cc2756b9f8a8ab + CONTRIB_REPO_SHA: fd12b1d624fe44ca17d2c88c0ace39dc80db85df jobs: build: From 6f607cb9c2e659cad616f7651ac763d4eac208d3 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 3 Dec 2020 21:09:51 +0000 Subject: [PATCH 11/12] Update opentelemetry-api/src/opentelemetry/configuration/__init__.py Co-authored-by: Aaron Abbott --- opentelemetry-api/src/opentelemetry/configuration/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 5e3d3667dc1..cc57f908dd4 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -186,7 +186,7 @@ def _traced_request_attrs(self, instrumentation: str) -> List[str]: value = self._config_map.get(key, "") request_attrs = ( - [attr.strip() for attr in str.split(value, ",")] if value else [] # type: ignore + [attr.strip() for attr in value.split(",")] if value else [] # type: ignore ) return request_attrs From f5ccb50500b63bae3862c142b310aa4f7761dc04 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 4 Dec 2020 13:38:50 +0530 Subject: [PATCH 12/12] Fix mypy issue --- opentelemetry-api/src/opentelemetry/configuration/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index cc57f908dd4..5e3d3667dc1 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -186,7 +186,7 @@ def _traced_request_attrs(self, instrumentation: str) -> List[str]: value = self._config_map.get(key, "") request_attrs = ( - [attr.strip() for attr in value.split(",")] if value else [] # type: ignore + [attr.strip() for attr in str.split(value, ",")] if value else [] # type: ignore ) return request_attrs