From 78e7bac88273f0296fcfc3a5b869b90f40c3c23e Mon Sep 17 00:00:00 2001 From: Prashant Srivastava Date: Wed, 28 Aug 2024 15:46:11 -0700 Subject: [PATCH 1/5] add check to skip instrumentation of wsgi master process --- .../distro/aws_opentelemetry_configurator.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py index 188a25842..5e85a628e 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py @@ -65,6 +65,7 @@ DEFAULT_METRIC_EXPORT_INTERVAL = 60000.0 AWS_LAMBDA_FUNCTION_NAME_CONFIG = "AWS_LAMBDA_FUNCTION_NAME" AWS_XRAY_DAEMON_ADDRESS_CONFIG = "AWS_XRAY_DAEMON_ADDRESS" +OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED_CONFIG = "OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED" _logger: Logger = getLogger(__name__) @@ -85,7 +86,10 @@ class AwsOpenTelemetryConfigurator(_OTelSDKConfigurator): # pylint: disable=no-self-use @override def _configure(self, **kwargs): - _initialize_components() + if _is_defer_to_workers_enabled() and _is_wsgi_master_process(): + return + else: + _initialize_components() # The OpenTelemetry Authors code @@ -156,6 +160,18 @@ def _init_tracing( # END The OpenTelemetry Authors code +def _is_defer_to_workers_enabled(): + return os.environ.get(OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED_CONFIG, "false").strip().lower() == "true" + + +def _is_wsgi_master_process(): + if os.environ.get("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "false").lower() == "true": + return False + else: + os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"] = "true" + return True + + def _exclude_urls_for_instrumentations(): urls_to_exclude_instr = "SamplingTargets,GetSamplingRules" requests_excluded_urls = os.environ.pop("OTEL_PYTHON_REQUESTS_EXCLUDED_URLS", "") From 211a5b6db1a0b6d2c754f59bc7e25ed667d287a8 Mon Sep 17 00:00:00 2001 From: Prashant Srivastava Date: Wed, 28 Aug 2024 16:34:04 -0700 Subject: [PATCH 2/5] adding some comment. Plus a simple unit test to begin with. --- .../distro/aws_opentelemetry_configurator.py | 8 ++++++++ .../distro/test_aws_opentelementry_configurator.py | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py index 5e85a628e..fb2386cb7 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py @@ -165,6 +165,14 @@ def _is_defer_to_workers_enabled(): def _is_wsgi_master_process(): + # Since the auto-instrumentation loads whenever a process is created and due to known issues with instrumenting + # WSGI apps using OTel, we want to skip the instrumentation of master process. + # This function is used to identify if the current process is a WSGI server's master process or not. + # Typically, a WSGI fork process model server spawns a single master process and multiple worker processes. + # When the master process starts, we use an environment variable as a marker. Since child worker processes inherit + # the master process environment, checking this marker in worker will tell that master process has been seen. + # Note: calling this function more than once in the same master process will return incorrect result. + # So use carefully. if os.environ.get("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "false").lower() == "true": return False else: diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py index 080ee7c7e..63643a4f8 100644 --- a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py @@ -16,6 +16,7 @@ _customize_sampler, _customize_span_processors, _is_application_signals_enabled, + _is_wsgi_master_process, ) from amazon.opentelemetry.distro.aws_opentelemetry_distro import AwsOpenTelemetryDistro from amazon.opentelemetry.distro.aws_span_metrics_processor import AwsSpanMetricsProcessor @@ -305,6 +306,10 @@ def test_application_signals_exporter_provider(self): self.assertEqual("127.0.0.1:2000", exporter._udp_exporter._endpoint) os.environ.pop("AWS_LAMBDA_FUNCTION_NAME", None) + def test_is_wsgi_master_process_first_time(self): + self.assertTrue(_is_wsgi_master_process()) + self.assertEqual(os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"], "true") + def validate_distro_environ(): tc: TestCase = TestCase() From 5e24da4b383847e5a57b652a6039a14ef254bdc8 Mon Sep 17 00:00:00 2001 From: Prashant Srivastava Date: Thu, 29 Aug 2024 13:39:42 -0700 Subject: [PATCH 3/5] adding more unit tests --- .../test_aws_opentelementry_configurator.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py index 63643a4f8..13793681c 100644 --- a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py @@ -16,6 +16,7 @@ _customize_sampler, _customize_span_processors, _is_application_signals_enabled, + _is_defer_to_workers_enabled, _is_wsgi_master_process, ) from amazon.opentelemetry.distro.aws_opentelemetry_distro import AwsOpenTelemetryDistro @@ -306,9 +307,37 @@ def test_application_signals_exporter_provider(self): self.assertEqual("127.0.0.1:2000", exporter._udp_exporter._endpoint) os.environ.pop("AWS_LAMBDA_FUNCTION_NAME", None) + def test_is_defer_to_workers_enabled(self): + os.environ.setdefault("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", "True") + self.assertTrue(_is_defer_to_workers_enabled()) + os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + + os.environ.setdefault("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", "False") + self.assertFalse(_is_defer_to_workers_enabled()) + os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + self.assertFalse(_is_defer_to_workers_enabled()) + def test_is_wsgi_master_process_first_time(self): self.assertTrue(_is_wsgi_master_process()) self.assertEqual(os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"], "true") + os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) + + @patch("amazon.opentelemetry.distro.aws_opentelemetry_configurator._initialize_components") + def test_initialize_components_skipped_when_defer_to_workers_enabled(self, mock_initialize_components): + os.environ.setdefault("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", "True") + self.assertTrue(_is_defer_to_workers_enabled()) + mock_initialize_components.assert_not_called() + os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + + @patch("amazon.opentelemetry.distro.aws_opentelemetry_configurator._initialize_components") + def test_initialize_components_called_in_worker_when_deferred_enabled(self, mock_initialize_components): + os.environ.setdefault("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", "True") + os.environ.setdefault("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "true") + self.assertTrue(_is_defer_to_workers_enabled()) + self.assertFalse(_is_wsgi_master_process()) + mock_initialize_components.assert_called_once() + os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) def validate_distro_environ(): From 9e7adc9cc582b3872fd1fda009eb8a39df0e0667 Mon Sep 17 00:00:00 2001 From: Prashant Srivastava Date: Thu, 29 Aug 2024 14:48:38 -0700 Subject: [PATCH 4/5] lint fixes, test fixes, more unit tests --- .../distro/aws_opentelemetry_configurator.py | 8 +++----- .../distro/test_aws_opentelementry_configurator.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py index fb2386cb7..d288ffe50 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py @@ -88,8 +88,7 @@ class AwsOpenTelemetryConfigurator(_OTelSDKConfigurator): def _configure(self, **kwargs): if _is_defer_to_workers_enabled() and _is_wsgi_master_process(): return - else: - _initialize_components() + _initialize_components() # The OpenTelemetry Authors code @@ -175,9 +174,8 @@ def _is_wsgi_master_process(): # So use carefully. if os.environ.get("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "false").lower() == "true": return False - else: - os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"] = "true" - return True + os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"] = "true" + return True def _exclude_urls_for_instrumentations(): diff --git a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py index 13793681c..d8bfc5756 100644 --- a/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py +++ b/aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py @@ -323,11 +323,14 @@ def test_is_wsgi_master_process_first_time(self): os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) @patch("amazon.opentelemetry.distro.aws_opentelemetry_configurator._initialize_components") - def test_initialize_components_skipped_when_defer_to_workers_enabled(self, mock_initialize_components): + def test_initialize_components_skipped_in_master_when_deferred_enabled(self, mock_initialize_components): os.environ.setdefault("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", "True") + os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) self.assertTrue(_is_defer_to_workers_enabled()) + AwsOpenTelemetryConfigurator()._configure() mock_initialize_components.assert_not_called() os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) @patch("amazon.opentelemetry.distro.aws_opentelemetry_configurator._initialize_components") def test_initialize_components_called_in_worker_when_deferred_enabled(self, mock_initialize_components): @@ -335,10 +338,19 @@ def test_initialize_components_called_in_worker_when_deferred_enabled(self, mock os.environ.setdefault("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "true") self.assertTrue(_is_defer_to_workers_enabled()) self.assertFalse(_is_wsgi_master_process()) + AwsOpenTelemetryConfigurator()._configure() mock_initialize_components.assert_called_once() os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) + @patch("amazon.opentelemetry.distro.aws_opentelemetry_configurator._initialize_components") + def test_initialize_components_called_when_deferred_disabled(self, mock_initialize_components): + os.environ.pop("OTEL_AWS_PYTHON_DEFER_TO_WORKERS_ENABLED", None) + self.assertFalse(_is_defer_to_workers_enabled()) + AwsOpenTelemetryConfigurator()._configure() + mock_initialize_components.assert_called_once() + os.environ.pop("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", None) + def validate_distro_environ(): tc: TestCase = TestCase() From 7dffcdf0192f45f761e2e9cdcb11768ae7947be2 Mon Sep 17 00:00:00 2001 From: Prashant Srivastava Date: Thu, 29 Aug 2024 16:06:19 -0700 Subject: [PATCH 5/5] adding info logs --- .../opentelemetry/distro/aws_opentelemetry_configurator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py index d288ffe50..0ac1f69de 100644 --- a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py +++ b/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py @@ -87,6 +87,9 @@ class AwsOpenTelemetryConfigurator(_OTelSDKConfigurator): @override def _configure(self, **kwargs): if _is_defer_to_workers_enabled() and _is_wsgi_master_process(): + _logger.info( + "Skipping ADOT initialization since deferral to worker is enabled, and this is a master process." + ) return _initialize_components() @@ -173,8 +176,10 @@ def _is_wsgi_master_process(): # Note: calling this function more than once in the same master process will return incorrect result. # So use carefully. if os.environ.get("IS_WSGI_MASTER_PROCESS_ALREADY_SEEN", "false").lower() == "true": + _logger.info("pid %s identified as a worker process", str(os.getpid())) return False os.environ["IS_WSGI_MASTER_PROCESS_ALREADY_SEEN"] = "true" + _logger.info("pid %s identified as a master process", str(os.getpid())) return True