Reload propagators for distro config + Reorder propagators (#421)

## Issue
The [`OTEL_PROPAGATORS` configured in the
ADOT](https://github.com/aws-observability/aws-otel-python-instrumentation/blob/af71c4a100f0ae9c8d378db8487988344485b0c3/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py#L73)
do not actually take effect. The configured propagators are still
`tracecontext` and `baggage` which are the default ones configured by
OTel SDK. This causes ADOT to ignore the `x-amzn-trace-id` trace header
causing broken traces if there is only XRay format trace being
propagated.

## Root Cause
- The [`opentelemetry.propagate`
module](https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/propagate/__init__.py)
has [the logic to load the
propagators](https://github.com/open-telemetry/opentelemetry-python/blob/0a2df97a72561552fb72e41c6fd092578a5f43e9/opentelemetry-api/src/opentelemetry/propagate/__init__.py#L124-L163)
configured via the `OTEL_PROPAGATORS` env variable. If the env variable
is not set, then it sets up the default propagators.
- This module is loaded and run very early in the OTel startup logic
even before the ADOT distro code is run which sets the
`OTEL_PROPAGATORS` env variable. This point is too late to modify the
OTEL_PROPAGATORS env variable.

## Solution
Set the `OTEL_PROPAGATORS` env variable in ADOT distro **and** reload
the `opentelemetry.propagate` module (using
[importlib.reload](https://docs.python.org/3/library/importlib.html#importlib.reload))
which will force it to reinitialize the intended propagators.

## Related Note
With this fix, the ADOT configured propagators will take effect, which
currently does not include `baggage` and will break some existing
functionality that depends on baggage. So I'm also also updating the
propagators set by the ADOT to:
- include `baggage`
- drop the B3 propagators as [ADOT JS
did](aws-observability/aws-otel-js-instrumentation@82e563c#diff-02388c43665de519f206132ad186755c563709c77f6de9f919a4e70fc51abc25)
- reorder to `baggage, xray, tracecontext`. This will be the order we
follow for other ADOT SDKs too ([JS already in
progress](aws-observability/aws-otel-js-instrumentation#210)).




By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

Author: srprash

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_distro.py

@@ -1,5 +1,6 @@
 # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
+import importlib
 import os
 import sys
 from logging import Logger, getLogger
@@ -18,6 +19,7 @@
     OTEL_TRACES_SAMPLER,
 )
 from amazon.opentelemetry.distro.patches._instrumentation_patch import apply_instrumentation_patches
+from opentelemetry import propagate
 from opentelemetry.distro import OpenTelemetryDistro
 from opentelemetry.environment_variables import OTEL_PROPAGATORS, OTEL_PYTHON_ID_GENERATOR
 from opentelemetry.sdk.environment_variables import (
@@ -70,7 +72,16 @@ def _configure(self, **kwargs):
 
         os.environ.setdefault(OTEL_EXPORTER_OTLP_PROTOCOL, "http/protobuf")
 
-        os.environ.setdefault(OTEL_PROPAGATORS, "xray,tracecontext,b3,b3multi")
+        if os.environ.get(OTEL_PROPAGATORS, None) is None:
+            # xray is set after baggage in case xray propagator depends on the result of the baggage header extraction.
+            os.environ.setdefault(OTEL_PROPAGATORS, "baggage,xray,tracecontext")
+            # We need to explicitly reload the opentelemetry.propagate module here
+            # because this module initializes the default propagators when it loads very early in the chain.
+            # Without reloading the OTEL_PROPAGATOR config from this distro won't take any effect.
+            # It's a hack from our end until OpenTelemetry fixes this behavior for distros to
+            # override the default propagators.
+            importlib.reload(propagate)
+
         os.environ.setdefault(OTEL_PYTHON_ID_GENERATOR, "xray")
         os.environ.setdefault(
             OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION, "base2_exponential_bucket_histogram"

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py

@@ -1262,7 +1262,7 @@ def validate_distro_environ():
     tc.assertEqual(
         "base2_exponential_bucket_histogram", os.environ.get("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION")
     )
-    tc.assertEqual("xray,tracecontext,b3,b3multi", os.environ.get("OTEL_PROPAGATORS"))
+    tc.assertEqual("baggage,xray,tracecontext", os.environ.get("OTEL_PROPAGATORS"))
     tc.assertEqual("xray", os.environ.get("OTEL_PYTHON_ID_GENERATOR"))
 
     # Not set

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelemetry_distro.py

@@ -1,11 +1,15 @@
 # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
+import importlib
 import os
+import sys
 from importlib.metadata import PackageNotFoundError, version
 from unittest import TestCase
 from unittest.mock import patch
 
 from amazon.opentelemetry.distro.aws_opentelemetry_distro import AwsOpenTelemetryDistro
+from opentelemetry import propagate
+from opentelemetry.propagators.composite import CompositePropagator
 
 
 class TestAwsOpenTelemetryDistro(TestCase):
@@ -40,6 +44,9 @@ def setUp(self):
             if var in os.environ:
                 del os.environ[var]
 
+        # Preserve the original sys.path
+        self.original_sys_path = sys.path.copy()
+
     def tearDown(self):
         # Clear all env vars first
         for var in self.env_vars_to_check:
@@ -50,6 +57,9 @@ def tearDown(self):
         for var, value in self.env_vars_to_restore.items():
             os.environ[var] = value
 
+        # Restore the original sys.path
+        sys.path[:] = self.original_sys_path
+
     def test_package_available(self):
         try:
             version("aws-opentelemetry-distro")
@@ -65,7 +75,7 @@ def test_configure_sets_default_values(self, mock_super_configure, mock_apply_pa
 
         # Check that default values are set
         self.assertEqual(os.environ.get("OTEL_EXPORTER_OTLP_PROTOCOL"), "http/protobuf")
-        self.assertEqual(os.environ.get("OTEL_PROPAGATORS"), "xray,tracecontext,b3,b3multi")
+        self.assertEqual(os.environ.get("OTEL_PROPAGATORS"), "baggage,xray,tracecontext")
         self.assertEqual(os.environ.get("OTEL_PYTHON_ID_GENERATOR"), "xray")
         self.assertEqual(
             os.environ.get("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION"),
@@ -188,16 +198,15 @@ def test_configure_preserves_existing_env_vars(
     @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.apply_instrumentation_patches")
     @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.OpenTelemetryDistro._configure")
     @patch("os.getcwd")
-    @patch("sys.path", new_callable=list)
-    def test_configure_adds_cwd_to_sys_path(self, mock_sys_path, mock_getcwd, mock_super_configure, mock_apply_patches):
+    def test_configure_adds_cwd_to_sys_path(self, mock_getcwd, mock_super_configure, mock_apply_patches):
         """Test that _configure adds current working directory to sys.path"""
         mock_getcwd.return_value = "/test/working/directory"
 
         distro = AwsOpenTelemetryDistro()
         distro._configure()
 
         # Check that cwd was added to sys.path
-        self.assertIn("/test/working/directory", mock_sys_path)
+        self.assertIn("/test/working/directory", sys.path)
 
     @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.get_aws_region")
     @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.is_agent_observability_enabled")
@@ -224,3 +233,47 @@ def test_configure_with_agent_observability_endpoints_already_set(
         # And exporters are still set to otlp
         self.assertEqual(os.environ.get("OTEL_TRACES_EXPORTER"), "otlp")
         self.assertEqual(os.environ.get("OTEL_LOGS_EXPORTER"), "otlp")
+
+    @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.apply_instrumentation_patches")
+    @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.OpenTelemetryDistro._configure")
+    def test_user_defined_propagators(self, mock_super_configure, mock_apply_patches):
+        """Test that user-defined propagators are respected"""
+        # Set user-defined propagators
+        os.environ["OTEL_PROPAGATORS"] = "xray"
+
+        # Force the reload of the propagate module otherwise the above environment
+        # variable doesn't taker effect.
+        importlib.reload(propagate)
+
+        distro = AwsOpenTelemetryDistro()
+        distro._configure()
+
+        # Verify that user-defined propagators are preserved
+        propagators = propagate.get_global_textmap()
+        self.assertTrue(isinstance(propagators, CompositePropagator))
+        expected_propagators = ["AwsXRayPropagator"]
+        individual_propagators = propagators._propagators
+        self.assertEqual(1, len(individual_propagators))
+        actual_propagators = []
+        for prop in individual_propagators:
+            actual_propagators.append(type(prop).__name__)
+        self.assertEqual(expected_propagators, actual_propagators)
+
+    @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.apply_instrumentation_patches")
+    @patch("amazon.opentelemetry.distro.aws_opentelemetry_distro.OpenTelemetryDistro._configure")
+    def test_otel_propagators_added_when_not_user_defined(self, mock_super_configure, mock_apply_patches):
+        distro = AwsOpenTelemetryDistro()
+        distro._configure()
+
+        # Verify that the propagators are set correctly by ADOT
+        propagators = propagate.get_global_textmap()
+
+        self.assertTrue(isinstance(propagators, CompositePropagator))
+
+        expected_propagators = ["W3CBaggagePropagator", "AwsXRayPropagator", "TraceContextTextMapPropagator"]
+        individual_propagators = propagators._propagators
+        self.assertEqual(3, len(individual_propagators))
+        actual_propagators = []
+        for prop in individual_propagators:
+            actual_propagators.append(type(prop).__name__)
+        self.assertEqual(expected_propagators, actual_propagators)