Pass OTLP metadata as tuple instead of string (#1507)

Author: alrex

CHANGELOG.md

@@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#1486](https://github.com/open-telemetry/opentelemetry-python/pull/1486))
 - Recreate span on every run of a `start_as_current_span`-decorated function
   ([#1451](https://github.com/open-telemetry/opentelemetry-python/pull/1451))
+- `opentelemetry-exporter-otlp` Headers are now passed in as tuple as metadata, instead of a
+  string, which was incorrect.
+  ([#1507](https://github.com/open-telemetry/opentelemetry-python/pull/1507))
 
 ## [0.16b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.16b1) - 2020-11-26
 ### Added

exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py

@@ -151,7 +151,7 @@ def __init__(
         endpoint: Optional[str] = None,
         insecure: Optional[bool] = None,
         credentials: Optional[ChannelCredentials] = None,
-        headers: Optional[str] = None,
+        headers: Optional[Sequence] = None,
         timeout: Optional[int] = None,
         compression: str = None,
     ):
@@ -169,6 +169,10 @@ def __init__(
             insecure = False
 
         self._headers = headers or Configuration().EXPORTER_OTLP_HEADERS
+        if isinstance(self._headers, str):
+            self._headers = tuple(
+                tuple(item.split("=")) for item in self._headers.split(",")
+            )
         self._timeout = (
             timeout
             or Configuration().EXPORTER_OTLP_TIMEOUT

exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/metrics_exporter/__init__.py

@@ -139,7 +139,7 @@ def __init__(
         endpoint: Optional[str] = None,
         insecure: Optional[bool] = None,
         credentials: Optional[ChannelCredentials] = None,
-        headers: Optional[str] = None,
+        headers: Optional[Sequence] = None,
         timeout: Optional[int] = None,
     ):
         if insecure is None:

exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/trace_exporter/__init__.py

@@ -69,7 +69,7 @@ def __init__(
         endpoint: Optional[str] = None,
         insecure: Optional[bool] = None,
         credentials: Optional[ChannelCredentials] = None,
-        headers: Optional[str] = None,
+        headers: Optional[Sequence] = None,
         timeout: Optional[int] = None,
     ):
         if insecure is None:

exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py

@@ -73,7 +73,7 @@ def tearDown(self):
             "OTEL_EXPORTER_OTLP_METRIC_ENDPOINT": "collector:55680",
             "OTEL_EXPORTER_OTLP_METRIC_CERTIFICATE": THIS_DIR
             + "/fixtures/test.cert",
-            "OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1:value1;key2:value2",
+            "OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1=value1,key2=value2",
             "OTEL_EXPORTER_OTLP_METRIC_TIMEOUT": "10",
         },
     )
@@ -85,7 +85,7 @@ def test_env_variables(self, mock_exporter_mixin):
         _, kwargs = mock_exporter_mixin.call_args_list[0]
 
         self.assertEqual(kwargs["endpoint"], "collector:55680")
-        self.assertEqual(kwargs["headers"], "key1:value1;key2:value2")
+        self.assertEqual(kwargs["headers"], "key1=value1,key2=value2")
         self.assertEqual(kwargs["timeout"], 10)
         self.assertIsNotNone(kwargs["credentials"])
         self.assertIsInstance(kwargs["credentials"], ChannelCredentials)
@@ -102,6 +102,35 @@ def test_no_credentials_error(
         OTLPMetricsExporter(insecure=False)
         self.assertTrue(mock_ssl_channel.called)
 
+    @patch.dict(
+        "os.environ",
+        {"OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1=value1,key2=value2"},
+    )
+    @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
+    @patch("opentelemetry.exporter.otlp.exporter.secure_channel")
+    # pylint: disable=unused-argument
+    def test_otlp_headers_from_env(self, mock_ssl_channel, mock_secure):
+        exporter = OTLPMetricsExporter()
+        # pylint: disable=protected-access
+        self.assertEqual(
+            exporter._headers, (("key1", "value1"), ("key2", "value2")),
+        )
+        exporter = OTLPMetricsExporter(
+            headers=(("key3", "value3"), ("key4", "value4"))
+        )
+        # pylint: disable=protected-access
+        self.assertEqual(
+            exporter._headers, (("key3", "value3"), ("key4", "value4")),
+        )
+
+    @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
+    @patch("opentelemetry.exporter.otlp.exporter.secure_channel")
+    # pylint: disable=unused-argument
+    def test_otlp_headers(self, mock_ssl_channel, mock_secure):
+        exporter = OTLPMetricsExporter()
+        # pylint: disable=protected-access
+        self.assertIsNone(exporter._headers, None)
+
     @patch("opentelemetry.sdk.metrics.export.aggregate.time_ns")
     def test_translate_counter_export_record(self, mock_time_ns):
         mock_time_ns.configure_mock(**{"return_value": 1})

exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py

@@ -172,7 +172,7 @@ def tearDown(self):
             "OTEL_EXPORTER_OTLP_SPAN_ENDPOINT": "collector:55680",
             "OTEL_EXPORTER_OTLP_SPAN_CERTIFICATE": THIS_DIR
             + "/fixtures/test.cert",
-            "OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1:value1;key2:value2",
+            "OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1=value1,key2=value2",
             "OTEL_EXPORTER_OTLP_SPAN_TIMEOUT": "10",
         },
     )
@@ -184,7 +184,7 @@ def test_env_variables(self, mock_exporter_mixin):
         _, kwargs = mock_exporter_mixin.call_args_list[0]
 
         self.assertEqual(kwargs["endpoint"], "collector:55680")
-        self.assertEqual(kwargs["headers"], "key1:value1;key2:value2")
+        self.assertEqual(kwargs["headers"], "key1=value1,key2=value2")
         self.assertEqual(kwargs["timeout"], 10)
         self.assertIsNotNone(kwargs["credentials"])
         self.assertIsInstance(kwargs["credentials"], ChannelCredentials)
@@ -199,6 +199,35 @@ def test_no_credentials_error(
         OTLPSpanExporter(insecure=False)
         self.assertTrue(mock_ssl_channel.called)
 
+    @patch.dict(
+        "os.environ",
+        {"OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1=value1,key2=value2"},
+    )
+    @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
+    @patch("opentelemetry.exporter.otlp.exporter.secure_channel")
+    # pylint: disable=unused-argument
+    def test_otlp_headers_from_env(self, mock_ssl_channel, mock_secure):
+        exporter = OTLPSpanExporter()
+        # pylint: disable=protected-access
+        self.assertEqual(
+            exporter._headers, (("key1", "value1"), ("key2", "value2"))
+        )
+        exporter = OTLPSpanExporter(
+            headers=(("key3", "value3"), ("key4", "value4"))
+        )
+        # pylint: disable=protected-access
+        self.assertEqual(
+            exporter._headers, (("key3", "value3"), ("key4", "value4"))
+        )
+
+    @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
+    @patch("opentelemetry.exporter.otlp.exporter.secure_channel")
+    # pylint: disable=unused-argument
+    def test_otlp_headers(self, mock_ssl_channel, mock_secure):
+        exporter = OTLPSpanExporter()
+        # pylint: disable=protected-access
+        self.assertIsNone(exporter._headers, None)
+
     @patch("opentelemetry.exporter.otlp.exporter.expo")
     @patch("opentelemetry.exporter.otlp.exporter.sleep")
     def test_unavailable(self, mock_sleep, mock_expo):