Make setters and getters optional (#1690)

Co-authored-by: alrex <aboten@lightstep.com>

.github/workflows/test.yml

@@ -10,7 +10,7 @@ env:
   # Otherwise, set variable to the commit of your branch on
   # opentelemetry-python-contrib which is compatible with these Core repo
   # changes.
-  CONTRIB_REPO_SHA: 5bc0fa1611502be47a1f4eb550fe255e4b707ba1
+  CONTRIB_REPO_SHA: 0d12fa39523212e268ef435825af2039a876fd75
 
 jobs:
   build:

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v0.18b0...HEAD)
+- Make setters and getters optional
+  ([#1690](https://github.com/open-telemetry/opentelemetry-python/pull/1690))
 
 ### Added
 - Document how to work with fork process web server models(Gunicorn, uWSGI etc...)

docs/conf.py

@@ -104,7 +104,7 @@
     ("py:class", "opentelemetry.trace._LinkBase",),
     # TODO: Understand why sphinx is not able to find this local class
     ("py:class", "opentelemetry.propagators.textmap.TextMapPropagator",),
-    ("py:class", "opentelemetry.propagators.textmap.DictGetter",),
+    ("py:class", "opentelemetry.propagators.textmap.DefaultGetter",),
     ("any", "opentelemetry.propagators.textmap.TextMapPropagator.extract",),
     ("any", "opentelemetry.propagators.textmap.TextMapPropagator.inject",),
 ]

docs/examples/auto-instrumentation/README.rst

@@ -37,8 +37,7 @@ Manually instrumented server
     def server_request():
         with tracer.start_as_current_span(
             "server_request",
-            context=propagators.extract(DictGetter(), request.headers
-            ),
+            context=propagators.extract(request.headers),
         ):
             print(request.args.get("param"))
             return "served"

docs/examples/auto-instrumentation/client.py

@@ -37,7 +37,7 @@
 
     with tracer.start_as_current_span("client-server"):
         headers = {}
-        propagators.inject(dict.__setitem__, headers)
+        propagators.inject(headers)
         requested = get(
             "http://localhost:8082/server_request",
             params={"param": argv[1]},

docs/examples/auto-instrumentation/server_instrumented.py

@@ -17,7 +17,6 @@
 from opentelemetry import trace
 from opentelemetry.instrumentation.wsgi import collect_request_attributes
 from opentelemetry.propagate import extract
-from opentelemetry.propagators.textmap import DictGetter
 from opentelemetry.sdk.trace import TracerProvider
 from opentelemetry.sdk.trace.export import (
     ConsoleSpanExporter,
@@ -38,7 +37,7 @@
 def server_request():
     with tracer.start_as_current_span(
         "server_request",
-        context=extract(DictGetter(), request.headers),
+        context=extract(request.headers),
         kind=trace.SpanKind.SERVER,
         attributes=collect_request_attributes(request.environ),
     ):

docs/examples/datadog_exporter/client.py

@@ -47,7 +47,7 @@
 
     with tracer.start_as_current_span("client-server"):
         headers = {}
-        inject(dict.__setitem__, headers)
+        inject(headers)
         requested = get(
             "http://localhost:8082/server_request",
             params={"param": argv[1]},

docs/examples/django/client.py

@@ -36,7 +36,7 @@
 
     with tracer.start_as_current_span("client-server"):
         headers = {}
-        inject(dict.__setitem__, headers)
+        inject(headers)
         requested = get(
             "http://localhost:8000",
             params={"param": argv[1]},

opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py

@@ -31,9 +31,9 @@ class W3CBaggagePropagator(textmap.TextMapPropagator):
 
     def extract(
         self,
-        getter: textmap.Getter[textmap.TextMapPropagatorT],
-        carrier: textmap.TextMapPropagatorT,
+        carrier: textmap.CarrierT,
         context: typing.Optional[Context] = None,
+        getter: textmap.Getter = textmap.default_getter,
     ) -> Context:
         """Extract Baggage from the carrier.
 
@@ -73,9 +73,9 @@ def extract(
 
     def inject(
         self,
-        set_in_carrier: textmap.Setter[textmap.TextMapPropagatorT],
-        carrier: textmap.TextMapPropagatorT,
+        carrier: textmap.CarrierT,
         context: typing.Optional[Context] = None,
+        setter: textmap.Setter = textmap.default_setter,
     ) -> None:
         """Injects Baggage into the carrier.
 
@@ -87,7 +87,7 @@ def inject(
             return
 
         baggage_string = _format_baggage(baggage_entries)
-        set_in_carrier(carrier, self._BAGGAGE_HEADER_NAME, baggage_string)
+        setter.set(carrier, self._BAGGAGE_HEADER_NAME, baggage_string)
 
     @property
     def fields(self) -> typing.Set[str]:
@@ -103,8 +103,8 @@ def _format_baggage(baggage_entries: typing.Mapping[str, object]) -> str:
 
 
 def _extract_first_element(
-    items: typing.Optional[typing.Iterable[textmap.TextMapPropagatorT]],
-) -> typing.Optional[textmap.TextMapPropagatorT]:
+    items: typing.Optional[typing.Iterable[textmap.CarrierT]],
+) -> typing.Optional[textmap.CarrierT]:
     if items is None:
         return None
     return next(iter(items), None)

opentelemetry-api/src/opentelemetry/propagate/__init__.py

@@ -82,9 +82,9 @@ def example_route():
 
 
 def extract(
-    getter: textmap.Getter[textmap.TextMapPropagatorT],
-    carrier: textmap.TextMapPropagatorT,
+    carrier: textmap.CarrierT,
     context: typing.Optional[Context] = None,
+    getter: textmap.Getter = textmap.default_getter,
 ) -> Context:
     """Uses the configured propagator to extract a Context from the carrier.
 
@@ -99,26 +99,26 @@ def extract(
         context: an optional Context to use. Defaults to current
             context if not set.
     """
-    return get_global_textmap().extract(getter, carrier, context)
+    return get_global_textmap().extract(carrier, context, getter=getter)
 
 
 def inject(
-    set_in_carrier: textmap.Setter[textmap.TextMapPropagatorT],
-    carrier: textmap.TextMapPropagatorT,
+    carrier: textmap.CarrierT,
     context: typing.Optional[Context] = None,
+    setter: textmap.Setter = textmap.default_setter,
 ) -> None:
     """Uses the configured propagator to inject a Context into the carrier.
 
     Args:
-        set_in_carrier: A setter function that can set values
-            on the carrier.
         carrier: An object that contains a representation of HTTP
-            headers. Should be paired with set_in_carrier, which
+            headers. Should be paired with setter, which
             should know how to set header values on the carrier.
-        context: an optional Context to use. Defaults to current
+        context: An optional Context to use. Defaults to current
             context if not set.
+        setter: An optional `Setter` object that can set values
+            on the carrier.
     """
-    get_global_textmap().inject(set_in_carrier, carrier, context)
+    get_global_textmap().inject(carrier, context=context, setter=setter)
 
 
 try:

opentelemetry-api/src/opentelemetry/propagators/composite.py

@@ -35,9 +35,9 @@ def __init__(
 
     def extract(
         self,
-        getter: textmap.Getter[textmap.TextMapPropagatorT],
-        carrier: textmap.TextMapPropagatorT,
+        carrier: textmap.CarrierT,
         context: typing.Optional[Context] = None,
+        getter: textmap.Getter = textmap.default_getter,
     ) -> Context:
         """Run each of the configured propagators with the given context and carrier.
         Propagators are run in the order they are configured, if multiple
@@ -47,14 +47,14 @@ def extract(
         See `opentelemetry.propagators.textmap.TextMapPropagator.extract`
         """
         for propagator in self._propagators:
-            context = propagator.extract(getter, carrier, context)
+            context = propagator.extract(carrier, context, getter=getter)
         return context  # type: ignore
 
     def inject(
         self,
-        set_in_carrier: textmap.Setter[textmap.TextMapPropagatorT],
-        carrier: textmap.TextMapPropagatorT,
+        carrier: textmap.CarrierT,
         context: typing.Optional[Context] = None,
+        setter: textmap.Setter = textmap.default_setter,
     ) -> None:
         """Run each of the configured propagators with the given context and carrier.
         Propagators are run in the order they are configured, if multiple
@@ -64,7 +64,7 @@ def inject(
         See `opentelemetry.propagators.textmap.TextMapPropagator.inject`
         """
         for propagator in self._propagators:
-            propagator.inject(set_in_carrier, carrier, context)
+            propagator.inject(carrier, context, setter=setter)
 
     @property
     def fields(self) -> typing.Set[str]:

opentelemetry-api/src/opentelemetry/propagators/textmap.py

@@ -17,19 +17,18 @@
 
 from opentelemetry.context.context import Context
 
-TextMapPropagatorT = typing.TypeVar("TextMapPropagatorT")
+CarrierT = typing.TypeVar("CarrierT")
 CarrierValT = typing.Union[typing.List[str], str]
 
-Setter = typing.Callable[[TextMapPropagatorT, str, str], None]
 
-
-class Getter(typing.Generic[TextMapPropagatorT]):
+class Getter(abc.ABC):
     """This class implements a Getter that enables extracting propagated
     fields from a carrier.
     """
 
+    @abc.abstractmethod
     def get(
-        self, carrier: TextMapPropagatorT, key: str
+        self, carrier: CarrierT, key: str
     ) -> typing.Optional[typing.List[str]]:
         """Function that can retrieve zero
         or more values from the carrier. In the case that
@@ -42,9 +41,9 @@ def get(
         Returns: first value of the propagation key or None if the key doesn't
                 exist.
         """
-        raise NotImplementedError()
 
-    def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]:
+    @abc.abstractmethod
+    def keys(self, carrier: CarrierT) -> typing.List[str]:
         """Function that can retrieve all the keys in a carrier object.
 
         Args:
@@ -53,17 +52,33 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]:
         Returns:
             list of keys from the carrier.
         """
-        raise NotImplementedError()
 
 
-class DictGetter(Getter[typing.Dict[str, CarrierValT]]):
-    def get(
-        self, carrier: typing.Dict[str, CarrierValT], key: str
+class Setter(abc.ABC):
+    """This class implements a Setter that enables injecting propagated
+    fields into a carrier.
+    """
+
+    @abc.abstractmethod
+    def set(self, carrier: CarrierT, key: str, value: str) -> None:
+        """Function that can set a value into a carrier""
+
+        Args:
+            carrier: An object which contains values that are used to
+                    construct a Context.
+            key: key of a field in carrier.
+            value: value for a field in carrier.
+        """
+
+
+class DefaultGetter(Getter):
+    def get(  # type: ignore
+        self, carrier: typing.Mapping[str, CarrierValT], key: str
     ) -> typing.Optional[typing.List[str]]:
         """Getter implementation to retrieve a value from a dictionary.
 
         Args:
-            carrier: dictionary in which header
+            carrier: dictionary in which to get value
             key: the key used to get the value
         Returns:
             A list with a single string with the value if it exists, else None.
@@ -75,11 +90,36 @@ def get(
             return list(val)
         return [val]
 
-    def keys(self, carrier: typing.Dict[str, CarrierValT]) -> typing.List[str]:
+    def keys(  # type: ignore
+        self, carrier: typing.Dict[str, CarrierValT]
+    ) -> typing.List[str]:
         """Keys implementation that returns all keys from a dictionary."""
         return list(carrier.keys())
 
 
+default_getter = DefaultGetter()
+
+
+class DefaultSetter(Setter):
+    def set(  # type: ignore
+        self,
+        carrier: typing.MutableMapping[str, CarrierValT],
+        key: str,
+        value: CarrierValT,
+    ) -> None:
+        """Setter implementation to set a value into a dictionary.
+
+        Args:
+            carrier: dictionary in which to set value
+            key: the key used to set the value
+            value: the value to set
+        """
+        carrier[key] = value
+
+
+default_setter = DefaultSetter()
+
+
 class TextMapPropagator(abc.ABC):
     """This class provides an interface that enables extracting and injecting
     context into headers of HTTP requests. HTTP frameworks and clients
@@ -92,9 +132,9 @@ class TextMapPropagator(abc.ABC):
     @abc.abstractmethod
     def extract(
         self,
-        getter: Getter[TextMapPropagatorT],
-        carrier: TextMapPropagatorT,
+        carrier: CarrierT,
         context: typing.Optional[Context] = None,
+        getter: Getter = default_getter,
     ) -> Context:
         """Create a Context from values in the carrier.
 
@@ -120,25 +160,25 @@ def extract(
     @abc.abstractmethod
     def inject(
         self,
-        set_in_carrier: Setter[TextMapPropagatorT],
-        carrier: TextMapPropagatorT,
+        carrier: CarrierT,
         context: typing.Optional[Context] = None,
+        setter: Setter = default_setter,
     ) -> None:
         """Inject values from a Context into a carrier.
 
         inject enables the propagation of values into HTTP clients or
         other objects which perform an HTTP request. Implementations
-        should use the set_in_carrier method to set values on the
+        should use the `Setter` 's set method to set values on the
         carrier.
 
         Args:
-            set_in_carrier: A setter function that can set values
-                on the carrier.
             carrier: An object that a place to define HTTP headers.
-                Should be paired with set_in_carrier, which should
+                Should be paired with setter, which should
                 know how to set header values on the carrier.
             context: an optional Context to use. Defaults to current
                 context if not set.
+            setter: An optional `Setter` object that can set values
+                on the carrier.
 
         """