Add a more informative error message when parsing ENV headers (#3044)

* Add a more informative error message when parsing ENV headers

Also, rename the function to make it clear that this parsing is specific
to headers provided via ENV variables.

Fixes #2376

* Use parse_env_headers in metrics and logs as well

* Fix lint

* Fix mypy

Author: ocelotl

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- Rename parse_headers to parse_env_headers and improve error message
+  ([#2376](https://github.com/open-telemetry/opentelemetry-python/pull/2376))
 - Add url decode values from OTEL_RESOURCE_ATTRIBUTES
   ([#3046](https://github.com/open-telemetry/opentelemetry-python/pull/3046))
 - Fixed circular dependency issue with custom samplers

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

@@ -52,8 +52,8 @@
     OTEL_EXPORTER_OTLP_TIMEOUT,
 )
 from opentelemetry.sdk.resources import Resource as SDKResource
-from opentelemetry.util.re import parse_headers
 from opentelemetry.sdk.metrics.export import MetricsData
+from opentelemetry.util.re import parse_env_headers
 
 logger = getLogger(__name__)
 SDKDataT = TypeVar("SDKDataT")
@@ -247,7 +247,7 @@ def __init__(
 
         self._headers = headers or environ.get(OTEL_EXPORTER_OTLP_HEADERS)
         if isinstance(self._headers, str):
-            temp_headers = parse_headers(self._headers)
+            temp_headers = parse_env_headers(self._headers)
             self._headers = tuple(temp_headers.items())
         elif isinstance(self._headers, dict):
             self._headers = tuple(self._headers.items())

exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py

@@ -42,7 +42,7 @@
 from opentelemetry.exporter.otlp.proto.http._log_exporter.encoder import (
     _ProtobufEncoder,
 )
-from opentelemetry.util.re import parse_headers
+from opentelemetry.util.re import parse_env_headers
 
 
 _logger = logging.getLogger(__name__)
@@ -86,7 +86,7 @@ def __init__(
             OTEL_EXPORTER_OTLP_CERTIFICATE, True
         )
         headers_string = environ.get(OTEL_EXPORTER_OTLP_HEADERS, "")
-        self._headers = headers or parse_headers(headers_string)
+        self._headers = headers or parse_env_headers(headers_string)
         self._timeout = timeout or int(
             environ.get(OTEL_EXPORTER_OTLP_TIMEOUT, DEFAULT_TIMEOUT)
         )

exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py

@@ -64,7 +64,7 @@
     Sum,
 )
 from opentelemetry.sdk.resources import Resource as SDKResource
-from opentelemetry.util.re import parse_headers
+from opentelemetry.util.re import parse_env_headers
 
 import backoff
 import requests
@@ -119,7 +119,7 @@ def __init__(
             OTEL_EXPORTER_OTLP_METRICS_HEADERS,
             environ.get(OTEL_EXPORTER_OTLP_HEADERS, ""),
         )
-        self._headers = headers or parse_headers(headers_string)
+        self._headers = headers or parse_env_headers(headers_string)
         self._timeout = timeout or int(
             environ.get(
                 OTEL_EXPORTER_OTLP_METRICS_TIMEOUT,

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

@@ -43,7 +43,7 @@
 from opentelemetry.exporter.otlp.proto.http.trace_exporter.encoder import (
     _ProtobufEncoder,
 )
-from opentelemetry.util.re import parse_headers
+from opentelemetry.util.re import parse_env_headers
 
 
 _logger = logging.getLogger(__name__)
@@ -94,7 +94,7 @@ def __init__(
             OTEL_EXPORTER_OTLP_TRACES_HEADERS,
             environ.get(OTEL_EXPORTER_OTLP_HEADERS, ""),
         )
-        self._headers = headers or parse_headers(headers_string)
+        self._headers = headers or parse_env_headers(headers_string)
         self._timeout = timeout or int(
             environ.get(
                 OTEL_EXPORTER_OTLP_TRACES_TIMEOUT,

exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py

@@ -220,7 +220,11 @@ def test_headers_parse_from_env(self):
 
             self.assertEqual(
                 cm.records[0].message,
-                "Header doesn't match the format: missingValue.",
+                (
+                    "Header format invalid! Header values in environment "
+                    "variables must be URL encoded per the OpenTelemetry "
+                    "Protocol Exporter specification: missingValue"
+                ),
             )
 
     @patch.object(requests.Session, "post")

exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py

@@ -189,7 +189,11 @@ def test_headers_parse_from_env(self):
 
             self.assertEqual(
                 cm.records[0].message,
-                "Header doesn't match the format: missingValue.",
+                (
+                    "Header format invalid! Header values in environment "
+                    "variables must be URL encoded per the OpenTelemetry "
+                    "Protocol Exporter specification: missingValue"
+                ),
             )
 
     # pylint: disable=no-self-use

opentelemetry-api/src/opentelemetry/util/re.py

@@ -12,34 +12,48 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import logging
+from logging import getLogger
 from re import compile, split
 from typing import Dict, List, Mapping
 from urllib.parse import unquote
 
-_logger = logging.getLogger(__name__)
+from deprecated import deprecated
 
+_logger = getLogger(__name__)
 
-# https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables
+# The following regexes reference this spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables
+
+# Optional whitespace
 _OWS = r"[ \t]*"
-# A key contains one or more US-ASCII character except CTLs or separators.
+# A key contains printable US-ASCII characters except: SP and "(),/:;<=>?@[\]{}
 _KEY_FORMAT = (
     r"[\x21\x23-\x27\x2a\x2b\x2d\x2e\x30-\x39\x41-\x5a\x5e-\x7a\x7c\x7e]+"
 )
-# A value contains a URL encoded UTF-8 string.
+# A value contains a URL-encoded UTF-8 string. The encoded form can contain any
+# printable US-ASCII characters (0x20-0x7f) other than SP, DEL, and ",;/
 _VALUE_FORMAT = r"[\x21\x23-\x2b\x2d-\x3a\x3c-\x5b\x5d-\x7e]*"
+# A key-value is key=value, with optional whitespace surrounding key and value
 _KEY_VALUE_FORMAT = rf"{_OWS}{_KEY_FORMAT}{_OWS}={_OWS}{_VALUE_FORMAT}{_OWS}"
+
 _HEADER_PATTERN = compile(_KEY_VALUE_FORMAT)
 _DELIMITER_PATTERN = compile(r"[ \t]*,[ \t]*")
 
 _BAGGAGE_PROPERTY_FORMAT = rf"{_KEY_VALUE_FORMAT}|{_OWS}{_KEY_FORMAT}{_OWS}"
 
 
 # pylint: disable=invalid-name
+
+
+@deprecated(version="1.15.0", reason="You should use parse_env_headers")  # type: ignore
 def parse_headers(s: str) -> Mapping[str, str]:
+    return parse_env_headers(s)
+
+
+def parse_env_headers(s: str) -> Mapping[str, str]:
     """
-    Parse ``s`` (a ``str`` instance containing HTTP headers). Uses W3C Baggage
-    HTTP header format https://www.w3.org/TR/baggage/#baggage-http-header-format, except that
+    Parse ``s``, which is a ``str`` instance containing HTTP headers encoded
+    for use in ENV variables per the W3C Baggage HTTP header format at
+    https://www.w3.org/TR/baggage/#baggage-http-header-format, except that
     additional semi-colon delimited metadata is not supported.
     """
     headers: Dict[str, str] = {}
@@ -49,7 +63,11 @@ def parse_headers(s: str) -> Mapping[str, str]:
             continue
         match = _HEADER_PATTERN.fullmatch(header.strip())
         if not match:
-            _logger.warning("Header doesn't match the format: %s.", header)
+            _logger.warning(
+                "Header format invalid! Header values in environment variables must be "
+                "URL encoded per the OpenTelemetry Protocol Exporter specification: %s",
+                header,
+            )
             continue
         # value may contain any number of `=`
         name, value = match.string.split("=", 1)

opentelemetry-api/tests/util/test_re.py

@@ -16,11 +16,11 @@
 
 import unittest
 
-from opentelemetry.util.re import parse_headers
+from opentelemetry.util.re import parse_env_headers
 
 
 class TestParseHeaders(unittest.TestCase):
-    def test_parse_headers(self):
+    def test_parse_env_headers(self):
         inp = [
             # invalid header name
             ("=value", [], True),
@@ -63,10 +63,12 @@ def test_parse_headers(self):
             s, expected, warn = case
             if warn:
                 with self.assertLogs(level="WARNING") as cm:
-                    self.assertEqual(parse_headers(s), dict(expected))
+                    self.assertEqual(parse_env_headers(s), dict(expected))
                     self.assertTrue(
-                        "Header doesn't match the format:"
+                        "Header format invalid! Header values in environment "
+                        "variables must be URL encoded per the OpenTelemetry "
+                        "Protocol Exporter specification:"
                         in cm.records[0].message,
                     )
             else:
-                self.assertEqual(parse_headers(s), dict(expected))
+                self.assertEqual(parse_env_headers(s), dict(expected))