From 8020463015b5353c4ee6e1e2d4541d35acfdee20 Mon Sep 17 00:00:00 2001 From: "mxiamxia@gmail.com" Date: Sun, 13 Jul 2025 13:25:39 -0700 Subject: [PATCH] Fix Starlette auto-instrumentation and botocore propagator configuration - Enable Starlette auto-instrumentation by removing it from disabled list - Add patch to relax Starlette version constraint (>=0.13, <0.15 to >=0.13) - Fix botocore instrumentation to use global propagator instead of hardcoded XRay --- .../distro/aws_opentelemetry_distro.py | 2 +- .../distro/patches/_botocore_patches.py | 23 ++++ .../distro/patches/_instrumentation_patch.py | 10 ++ .../distro/patches/_starlette_patches.py | 33 ++++++ .../distro/patches/test_starlette_patches.py | 108 ++++++++++++++++++ .../distro/test_aws_opentelemetry_distro.py | 2 +- .../distro/test_instrumentation_patch.py | 81 +++++++++++++ 7 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_starlette_patches.py create mode 100644 aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/patches/test_starlette_patches.py diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py index 6633a78f7..35a41780d 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py @@ -119,7 +119,7 @@ def _configure(self, **kwargs): os.environ.setdefault( OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, "http,sqlalchemy,psycopg2,pymysql,sqlite3,aiopg,asyncpg,mysql_connector," - "urllib3,requests,starlette,system_metrics,google-genai", + "urllib3,requests,system_metrics,google-genai", ) # Set logging auto instrumentation default diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_botocore_patches.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_botocore_patches.py index 7eb9a2066..8e621795b 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_botocore_patches.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_botocore_patches.py @@ -45,6 +45,7 @@ ) from opentelemetry.instrumentation.botocore.utils import get_server_attributes from opentelemetry.instrumentation.utils import is_instrumentation_enabled, suppress_http_instrumentation +from opentelemetry.propagate import get_global_textmap from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.span import Span @@ -54,6 +55,7 @@ def _apply_botocore_instrumentation_patches() -> None: Adds patches to provide additional support and Java parity for Kinesis, S3, and SQS. """ + _apply_botocore_propagator_patch() _apply_botocore_api_call_patch() _apply_botocore_kinesis_patch() _apply_botocore_s3_patch() @@ -66,6 +68,27 @@ def _apply_botocore_instrumentation_patches() -> None: _apply_botocore_dynamodb_patch() +# Known issue in OpenTelemetry upstream botocore auto-instrumentation +# TODO: Contribute fix upstream and remove from ADOT patch after the contribution +def _apply_botocore_propagator_patch() -> None: + """Botocore instrumentation patch for propagator + + Changes the default propagator from AwsXRayPropagator to the global propagator. + This allows the propagator to be configured via OTEL_PROPAGATORS environment variable. + """ + # Store the original __init__ method + original_init = BotocoreInstrumentor.__init__ + + def patched_init(self): + # Call the original __init__ + original_init(self) + # Replace the propagator with the global one + self.propagator = get_global_textmap() + + # Apply the patch + BotocoreInstrumentor.__init__ = patched_init + + def _apply_botocore_lambda_patch() -> None: """Botocore instrumentation patch for Lambda diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_instrumentation_patch.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_instrumentation_patch.py index 9a5f4974b..df066bca2 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_instrumentation_patch.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_instrumentation_patch.py @@ -61,6 +61,16 @@ def apply_instrumentation_patches() -> None: _apply_botocore_instrumentation_patches() + if is_installed("starlette"): + # pylint: disable=import-outside-toplevel + # Delay import to only occur if patches is safe to apply (e.g. the instrumented library is installed). + from amazon.opentelemetry.distro.patches._starlette_patches import _apply_starlette_instrumentation_patches + + # Starlette auto-instrumentation v0.54b includes a strict dependency version check + # This restriction was removed in v1.34.0/0.55b0. Applying temporary patch for Genesis launch + # TODO: Remove patch after syncing with upstream v1.34.0 or later + _apply_starlette_instrumentation_patches() + # No need to check if library is installed as this patches opentelemetry.sdk, # which must be installed for the distro to work at all. _apply_resource_detector_patches() diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_starlette_patches.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_starlette_patches.py new file mode 100644 index 000000000..b3bcd624b --- /dev/null +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/patches/_starlette_patches.py @@ -0,0 +1,33 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +# Modifications Copyright The OpenTelemetry Authors. Licensed under the Apache License 2.0 License. +from logging import Logger, getLogger +from typing import Collection + +_logger: Logger = getLogger(__name__) + + +# Upstream fix available in OpenTelemetry 1.34.0/0.55b0 (2025-06-04) +# Reference: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3456 +# TODO: Remove this patch after upgrading to version 1.34.0 or later +def _apply_starlette_instrumentation_patches() -> None: + """Apply patches to the Starlette instrumentation. + + This patch modifies the instrumentation_dependencies method in the starlette + instrumentation to loose an upper version constraint for auto-instrumentation + """ + try: + # pylint: disable=import-outside-toplevel + from opentelemetry.instrumentation.starlette import StarletteInstrumentor + + # Patch starlette dependencies version check + # Loose the upper check from ("starlette >= 0.13, <0.15",) + def patched_instrumentation_dependencies(self) -> Collection[str]: + return ("starlette >= 0.13",) + + # Apply the patch + StarletteInstrumentor.instrumentation_dependencies = patched_instrumentation_dependencies + + _logger.debug("Successfully patched Starlette instrumentation_dependencies method") + except Exception as exc: # pylint: disable=broad-except + _logger.warning("Failed to apply Starlette instrumentation patches: %s", exc) diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/patches/test_starlette_patches.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/patches/test_starlette_patches.py new file mode 100644 index 000000000..e0bbf4270 --- /dev/null +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/patches/test_starlette_patches.py @@ -0,0 +1,108 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from unittest import TestCase +from unittest.mock import MagicMock, patch + +from amazon.opentelemetry.distro.patches._starlette_patches import _apply_starlette_instrumentation_patches + + +class TestStarlettePatch(TestCase): + """Test the Starlette instrumentation patches.""" + + @patch("amazon.opentelemetry.distro.patches._starlette_patches._logger") + def test_starlette_patch_applied_successfully(self, mock_logger): + """Test that the Starlette instrumentation patch is applied successfully.""" + # Create a mock StarletteInstrumentor class + mock_instrumentor_class = MagicMock() + mock_instrumentor_class.__name__ = "StarletteInstrumentor" + + # Create a mock module + mock_starlette_module = MagicMock() + mock_starlette_module.StarletteInstrumentor = mock_instrumentor_class + + # Mock the import + with patch.dict("sys.modules", {"opentelemetry.instrumentation.starlette": mock_starlette_module}): + # Apply the patch + _apply_starlette_instrumentation_patches() + + # Verify the instrumentation_dependencies method was replaced + self.assertTrue(hasattr(mock_instrumentor_class, "instrumentation_dependencies")) + + # Test the patched method returns the expected value + mock_instance = MagicMock() + result = mock_instrumentor_class.instrumentation_dependencies(mock_instance) + self.assertEqual(result, ("starlette >= 0.13",)) + + # Verify logging + mock_logger.debug.assert_called_once_with( + "Successfully patched Starlette instrumentation_dependencies method" + ) + + @patch("amazon.opentelemetry.distro.patches._starlette_patches._logger") + def test_starlette_patch_handles_import_error(self, mock_logger): + """Test that the patch handles import errors gracefully.""" + # Mock the import to fail by removing the module + with patch.dict("sys.modules", {"opentelemetry.instrumentation.starlette": None}): + # This should not raise an exception + _apply_starlette_instrumentation_patches() + + # Verify warning was logged + mock_logger.warning.assert_called_once() + args = mock_logger.warning.call_args[0] + self.assertIn("Failed to apply Starlette instrumentation patches", args[0]) + + @patch("amazon.opentelemetry.distro.patches._starlette_patches._logger") + def test_starlette_patch_handles_attribute_error(self, mock_logger): + """Test that the patch handles attribute errors gracefully.""" + + # Create a metaclass that raises AttributeError when setting class attributes + class ErrorMeta(type): + def __setattr__(cls, name, value): + if name == "instrumentation_dependencies": + raise AttributeError("Cannot set attribute") + super().__setattr__(name, value) + + # Create a class with the error-raising metaclass + class MockStarletteInstrumentor(metaclass=ErrorMeta): + pass + + # Create a mock module + mock_starlette_module = MagicMock() + mock_starlette_module.StarletteInstrumentor = MockStarletteInstrumentor + + with patch.dict("sys.modules", {"opentelemetry.instrumentation.starlette": mock_starlette_module}): + # This should not raise an exception + _apply_starlette_instrumentation_patches() + + # Verify warning was logged + mock_logger.warning.assert_called_once() + args = mock_logger.warning.call_args[0] + self.assertIn("Failed to apply Starlette instrumentation patches", args[0]) + + def test_starlette_patch_logs_failure_with_no_logger_patch(self): # pylint: disable=no-self-use + """Test that the patch handles exceptions gracefully without logger mock.""" + # Mock the import to fail + with patch.dict("sys.modules", {"opentelemetry.instrumentation.starlette": None}): + # This should not raise an exception even without logger mock + _apply_starlette_instrumentation_patches() + + @patch("amazon.opentelemetry.distro.patches._starlette_patches._logger") + def test_starlette_patch_with_exception_during_import(self, mock_logger): + """Test that the patch handles exceptions during import.""" + + # Create a module that raises exception when accessing StarletteInstrumentor + class FailingModule: + @property + def StarletteInstrumentor(self): # pylint: disable=invalid-name + raise RuntimeError("Import failed") + + failing_module = FailingModule() + + with patch.dict("sys.modules", {"opentelemetry.instrumentation.starlette": failing_module}): + # This should not raise an exception + _apply_starlette_instrumentation_patches() + + # Verify warning was logged + mock_logger.warning.assert_called_once() + args = mock_logger.warning.call_args[0] + self.assertIn("Failed to apply Starlette instrumentation patches", args[0]) diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelemetry_distro.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelemetry_distro.py index 5a91c4a83..aed916b8c 100644 --- a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelemetry_distro.py +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelemetry_distro.py @@ -127,7 +127,7 @@ def test_configure_with_agent_observability_enabled( self.assertEqual( os.environ.get("OTEL_PYTHON_DISABLED_INSTRUMENTATIONS"), "http,sqlalchemy,psycopg2,pymysql,sqlite3,aiopg,asyncpg,mysql_connector," - "urllib3,requests,starlette,system_metrics,google-genai", + "urllib3,requests,system_metrics,google-genai", ) self.assertEqual(os.environ.get("OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED"), "true") self.assertEqual(os.environ.get("OTEL_AWS_APPLICATION_SIGNALS_ENABLED"), "false") diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_instrumentation_patch.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_instrumentation_patch.py index 857c13265..57dbdf6d8 100644 --- a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_instrumentation_patch.py +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_instrumentation_patch.py @@ -16,6 +16,7 @@ ) from opentelemetry.instrumentation.botocore import BotocoreInstrumentor from opentelemetry.instrumentation.botocore.extensions import _KNOWN_EXTENSIONS +from opentelemetry.propagate import get_global_textmap from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.span import Span @@ -80,14 +81,18 @@ def _run_patch_behaviour_tests(self): # Validate unpatched upstream behaviour - important to detect upstream changes that may break instrumentation self._test_unpatched_botocore_instrumentation() + self._test_unpatched_botocore_propagator() self._test_unpatched_gevent_instrumentation() + self._test_unpatched_starlette_instrumentation() # Apply patches apply_instrumentation_patches() # Validate patched upstream behaviour - important to detect downstream changes that may break instrumentation self._test_patched_botocore_instrumentation() + self._test_patched_botocore_propagator() self._test_unpatched_gevent_instrumentation() + self._test_patched_starlette_instrumentation() # Test setup to check whether only these two modules get patched by gevent monkey os.environ[AWS_GEVENT_PATCH_MODULES] = "os, ssl" @@ -123,6 +128,8 @@ def _run_patch_mechanism_tests(self): self._reset_mocks() self._test_resource_detector_patches() self._reset_mocks() + self._test_starlette_installed_flag() + self._reset_mocks() def _test_unpatched_botocore_instrumentation(self): # Kinesis @@ -567,6 +574,80 @@ def _test_resource_detector_patches(self): # Verify SSL context was created with correct CA file mock_ssl.assert_called_once_with(cafile="/var/run/secrets/kubernetes.io/serviceaccount/ca.crt") + def _test_unpatched_botocore_propagator(self): + """Test that BotocoreInstrumentor uses its own propagator by default.""" + # Create a fresh instrumentor to test its initial state + test_instrumentor = BotocoreInstrumentor() + # Check that it has its own propagator (not the global one) + self.assertIsNotNone(test_instrumentor.propagator) + # The default propagator should not be the global propagator initially + # This test ensures upstream hasn't changed their default behavior + + def _test_patched_botocore_propagator(self): + """Test that BotocoreInstrumentor uses global propagator after patching.""" + # Create a new instrumentor after patches have been applied + test_instrumentor = BotocoreInstrumentor() + # After patching, the propagator should be the global one + self.assertEqual(test_instrumentor.propagator, get_global_textmap()) + + def _test_unpatched_starlette_instrumentation(self): + """Test unpatched starlette instrumentation dependencies.""" + try: + # pylint: disable=import-outside-toplevel + from opentelemetry.instrumentation.starlette import StarletteInstrumentor + + # Store original method to verify it hasn't been patched yet + original_deps = StarletteInstrumentor.instrumentation_dependencies + # Create an instance to test the method + instrumentor = StarletteInstrumentor() + deps = original_deps(instrumentor) + # Default should have version constraint + self.assertEqual(deps, ("starlette >= 0.13, <0.15",)) + except ImportError: + # If starlette instrumentation is not installed, skip this test + pass + + def _test_patched_starlette_instrumentation(self): + """Test patched starlette instrumentation dependencies.""" + try: + # pylint: disable=import-outside-toplevel + from opentelemetry.instrumentation.starlette import StarletteInstrumentor + + # After patching, the version constraint should be relaxed + instrumentor = StarletteInstrumentor() + deps = instrumentor.instrumentation_dependencies() + self.assertEqual(deps, ("starlette >= 0.13",)) + except ImportError: + # If starlette instrumentation is not installed, skip this test + pass + + def _test_starlette_installed_flag(self): # pylint: disable=no-self-use + """Test that starlette patches are only applied when starlette is installed.""" + with patch( + "amazon.opentelemetry.distro.patches._starlette_patches._apply_starlette_instrumentation_patches" + ) as mock_apply_patches: + # Test when starlette is not installed + with patch( + "amazon.opentelemetry.distro.patches._instrumentation_patch.is_installed", return_value=False + ) as mock_is_installed: + apply_instrumentation_patches() + # Check that is_installed was called for starlette + mock_is_installed.assert_any_call("starlette") + # Patches should not be applied when starlette is not installed + mock_apply_patches.assert_not_called() + + mock_apply_patches.reset_mock() + + # Test when starlette is installed + with patch( + "amazon.opentelemetry.distro.patches._instrumentation_patch.is_installed", return_value=True + ) as mock_is_installed: + apply_instrumentation_patches() + # Check that is_installed was called for starlette + mock_is_installed.assert_any_call("starlette") + # Patches should be applied when starlette is installed + mock_apply_patches.assert_called() + def _reset_mocks(self): for method_patch in self.method_patches.values(): method_patch.reset_mock()