address feedback

Author: jj22ee

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/sampler/aws_xray_sampling_client.py renamed to aws-opentelemetry-distro/src/amazon/opentelemetry/distro/sampler/_aws_xray_sampling_client.py

@@ -5,12 +5,12 @@
 
 import requests
 
-from amazon.opentelemetry.distro.sampler.sampling_rule import SamplingRule
+from amazon.opentelemetry.distro.sampler._sampling_rule import _SamplingRule
 
 _logger = getLogger(__name__)
 
 
-class AwsXRaySamplingClient:
+class _AwsXRaySamplingClient:
     def __init__(self, endpoint=None, log_level=None):
         # Override default log level
         if log_level is not None:
@@ -27,22 +27,22 @@ def get_sampling_rules(self):
         try:
             xray_response = requests.post(url=self.__get_sampling_rules_endpoint, headers=headers, timeout=20)
             if xray_response is None:
-                raise ValueError("GetSamplingRules response is None")
+                _logger.error("GetSamplingRules response is None")
+                return []
             sampling_rules_response = xray_response.json()
             if "SamplingRuleRecords" not in sampling_rules_response:
-                raise ValueError(
-                    f"SamplingRuleRecords is missing in getSamplingRules response:{sampling_rules_response}"
+                _logger.error(
+                    "SamplingRuleRecords is missing in getSamplingRules response: %s", sampling_rules_response
                 )
+                return []
 
             sampling_rules_records = sampling_rules_response["SamplingRuleRecords"]
             for record in sampling_rules_records:
-                sampling_rules.append(SamplingRule(**record["SamplingRule"]))
+                sampling_rules.append(_SamplingRule(**record["SamplingRule"]))
 
         except requests.exceptions.RequestException as req_err:
-            _logger.exception("Request error occurred: %s", req_err)
+            _logger.error("Request error occurred: %s", req_err)
         except json.JSONDecodeError as json_err:
-            _logger.exception("Error in decoding JSON response: %s", json_err)
-        except ValueError as ex:
-            _logger.exception("Exception occurred: %s", ex)
+            _logger.error("Error in decoding JSON response: %s", json_err)
 
         return sampling_rules

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/sampler/sampling_rule.py renamed to aws-opentelemetry-distro/src/amazon/opentelemetry/distro/sampler/_sampling_rule.py

@@ -4,7 +4,7 @@
 
 # Disable snake_case naming style so this class can match the sampling rules response from X-Ray
 # pylint: disable=invalid-name
-class SamplingRule:
+class _SamplingRule:
     def __init__(
         self,
         Attributes=None,

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/sampler/aws_xray_remote_sampler.py

@@ -5,7 +5,9 @@
 from threading import Timer
 from typing import Optional, Sequence
 
-from amazon.opentelemetry.distro.sampler.aws_xray_sampling_client import AwsXRaySamplingClient
+from typing_extensions import override
+
+from amazon.opentelemetry.distro.sampler._aws_xray_sampling_client import _AwsXRaySamplingClient
 from opentelemetry.context import Context
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.sdk.trace.sampling import ALWAYS_OFF, Sampler, SamplingResult
@@ -15,8 +17,8 @@
 
 _logger = getLogger(__name__)
 
-DEFAULT_RULES_POLLING_INTERVAL = 300
-DEFAULT_TARGET_POLLING_INTERVAL = 10
+DEFAULT_RULES_POLLING_INTERVAL_SECONDS = 300
+DEFAULT_TARGET_POLLING_INTERVAL_SECONDS = 10
 DEFAULT_SAMPLING_PROXY_ENDPOINT = "http://127.0.0.1:2000"
 
 
@@ -25,36 +27,44 @@ class AwsXRayRemoteSampler(Sampler):
     Remote Sampler for OpenTelemetry that gets sampling configurations from AWS X-Ray
 
     Args:
-        resource: OpenTelemetry Resource (Optional)
+        resource: OpenTelemetry Resource (Required)
         endpoint: proxy endpoint for AWS X-Ray Sampling (Optional)
         polling_interval: Polling interval for getSamplingRules call (Optional)
         log_level: custom log level configuration for remote sampler (Optional)
     """
 
     __resource: Resource
     __polling_interval: int
-    __xray_client: AwsXRaySamplingClient
+    __xray_client: _AwsXRaySamplingClient
 
     def __init__(
         self,
-        resource=None,
+        resource: Resource,
         endpoint=DEFAULT_SAMPLING_PROXY_ENDPOINT,
-        polling_interval=DEFAULT_RULES_POLLING_INTERVAL,
+        polling_interval=DEFAULT_RULES_POLLING_INTERVAL_SECONDS,
         log_level=None,
     ):
         # Override default log level
         if log_level is not None:
             _logger.setLevel(log_level)
 
-        self.__xray_client = AwsXRaySamplingClient(endpoint, log_level=log_level)
+        self.__xray_client = _AwsXRaySamplingClient(endpoint, log_level=log_level)
         self.__polling_interval = polling_interval
 
         # pylint: disable=unused-private-member
-        self.__resource = resource
-
-        self.__start_sampling_rule_poller()
+        if resource is not None:
+            self.__resource = resource
+        else:
+            _logger.warning("OTel Resource provided is `None`. Defaulting to empty resource")
+            self.__resource = Resource.get_empty()
+
+        # Schedule the next rule poll now
+        self._timer = Timer(0, self.__start_sampling_rule_poller)
+        self._timer.daemon = True
+        self._timer.start()
 
     # pylint: disable=no-self-use
+    @override
     def should_sample(
         self,
         parent_context: Optional["Context"],
@@ -64,13 +74,14 @@ def should_sample(
         attributes: Attributes = None,
         links: Sequence["Link"] = None,
         trace_state: "TraceState" = None,
-    ) -> "SamplingResult":
+    ) -> SamplingResult:
         # TODO: add sampling functionality
         return ALWAYS_OFF.should_sample(
             self, parent_context, trace_id, name, kind=kind, attributes=attributes, links=links, trace_state=trace_state
         )
 
     # pylint: disable=no-self-use
+    @override
     def get_description(self) -> str:
         description = "AwsXRayRemoteSampler{remote sampling with AWS X-Ray}"
         return description

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/sampler/test_aws_xray_remote_sampler.py

@@ -7,7 +7,7 @@
 from opentelemetry.sdk.resources import Resource
 
 
-class AwsXRayRemoteSamplerTest(TestCase):
+class TestAwsXRayRemoteSampler(TestCase):
     def test_create_remote_sampler_with_empty_resource(self):
         rs = AwsXRayRemoteSampler(resource=Resource.get_empty())
         self.assertIsNotNone(rs._timer)
@@ -40,6 +40,6 @@ def test_create_remote_sampler_with_all_fields_populated(self):
         self.assertEqual(
             rs._AwsXRayRemoteSampler__xray_client._AwsXRaySamplingClient__get_sampling_rules_endpoint,
             "http://abc.com/GetSamplingRules",
-        )  # "http://127.0.0.1:2000"
+        )
         self.assertEqual(rs._AwsXRayRemoteSampler__resource.attributes["service.name"], "test-service-name")
         self.assertEqual(rs._AwsXRayRemoteSampler__resource.attributes["cloud.platform"], "test-cloud-platform")

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/sampler/test_aws_xray_sampling_client.py

@@ -6,27 +6,27 @@
 from unittest import TestCase
 from unittest.mock import patch
 
-from amazon.opentelemetry.distro.sampler.aws_xray_sampling_client import AwsXRaySamplingClient
+from amazon.opentelemetry.distro.sampler._aws_xray_sampling_client import _AwsXRaySamplingClient
 
-SAMPLING_CLIENT_LOGGER_NAME = "amazon.opentelemetry.distro.sampler.aws_xray_sampling_client"
+SAMPLING_CLIENT_LOGGER_NAME = "amazon.opentelemetry.distro.sampler._aws_xray_sampling_client"
 _logger = getLogger(SAMPLING_CLIENT_LOGGER_NAME)
 
 TEST_DIR = os.path.dirname(os.path.realpath(__file__))
 DATA_DIR = os.path.join(TEST_DIR, "data")
 
 
-class AwsXRaySamplingClientTest(TestCase):
+class TestAwsXRaySamplingClient(TestCase):
     @patch("requests.post")
     def test_get_no_sampling_rules(self, mock_post=None):
         mock_post.return_value.configure_mock(**{"json.return_value": {"SamplingRuleRecords": []}})
-        client = AwsXRaySamplingClient("http://127.0.0.1:2000")
+        client = _AwsXRaySamplingClient("http://127.0.0.1:2000")
         sampling_rules = client.get_sampling_rules()
         self.assertTrue(len(sampling_rules) == 0)
 
     @patch("requests.post")
     def test_get_invalid_response(self, mock_post=None):
         mock_post.return_value.configure_mock(**{"json.return_value": {}})
-        client = AwsXRaySamplingClient("http://127.0.0.1:2000")
+        client = _AwsXRaySamplingClient("http://127.0.0.1:2000")
         with self.assertLogs(_logger, level="ERROR"):
             sampling_rules = client.get_sampling_rules()
         self.assertTrue(len(sampling_rules) == 0)
@@ -36,6 +36,6 @@ def test_get_two_sampling_rules(self, mock_post=None):
         with open(f"{DATA_DIR}/get-sampling-rules-response-sample.json", encoding="UTF-8") as file:
             mock_post.return_value.configure_mock(**{"json.return_value": json.load(file)})
             file.close()
-        client = AwsXRaySamplingClient("http://127.0.0.1:2000")
+        client = _AwsXRaySamplingClient("http://127.0.0.1:2000")
         sampling_rules = client.get_sampling_rules()
         self.assertTrue(len(sampling_rules) == 3)