Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make setters and getters optional #1690

Merged
merged 29 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8ec609f
Remove setters and getters
ocelotl Mar 11, 2021
cfd4dc7
Remove code cleanup
ocelotl Mar 22, 2021
e00ef20
Revert "Remove code cleanup" and "Remove setters and getters"
ocelotl Mar 23, 2021
973ac4b
Try to implement with default getters and setters
ocelotl Mar 23, 2021
b4ec107
Fix mypy
ocelotl Mar 25, 2021
e4d1698
Fix changelog
ocelotl Mar 25, 2021
1586653
Rename to CarrierT
ocelotl Mar 25, 2021
017be2c
Rename set_in_carrier to setter
ocelotl Mar 25, 2021
2ca12cb
Remove unnecessary arguments, rename to Default
ocelotl Mar 25, 2021
e131478
Update SHA
ocelotl Mar 25, 2021
b298b56
Fix lint
ocelotl Mar 25, 2021
0cec680
Update SHA
ocelotl Mar 25, 2021
817599e
Update SHA
ocelotl Mar 25, 2021
df649ea
Update SHA
ocelotl Mar 25, 2021
f946e75
Update SHA
ocelotl Mar 25, 2021
f4ba4c5
Update SHA
ocelotl Mar 25, 2021
5ae9e98
Update SHA
ocelotl Mar 25, 2021
2a9b7b7
Update SHA
ocelotl Mar 25, 2021
3fa68d6
More fixes
ocelotl Mar 25, 2021
8a75bfa
Fix SHA
ocelotl Mar 25, 2021
c81c7d1
Update SHA
ocelotl Mar 26, 2021
94d9c1e
Fix SHA
ocelotl Mar 26, 2021
7291825
Update opentelemetry-api/src/opentelemetry/propagate/__init__.py
ocelotl Mar 26, 2021
533a926
Update opentelemetry-api/src/opentelemetry/propagators/textmap.py
ocelotl Mar 26, 2021
13f73ec
Update opentelemetry-api/src/opentelemetry/propagators/textmap.py
ocelotl Mar 26, 2021
a81ea56
Merge branch 'main' into issue_1644
Mar 26, 2021
0244b23
addressing docs comments
Mar 26, 2021
f515e2d
adding docs
Mar 26, 2021
5be5eac
review feedback
Mar 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",),
]
Expand Down
3 changes: 1 addition & 2 deletions docs/examples/auto-instrumentation/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/auto-instrumentation/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand Down
3 changes: 1 addition & 2 deletions docs/examples/auto-instrumentation/server_instrumented.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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),
):
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/datadog_exporter/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/django/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Expand All @@ -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]:
Expand All @@ -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)
20 changes: 10 additions & 10 deletions opentelemetry-api/src/opentelemetry/propagate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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:
Expand Down
12 changes: 6 additions & 6 deletions opentelemetry-api/src/opentelemetry/propagators/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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]:
Expand Down
82 changes: 61 additions & 21 deletions opentelemetry-api/src/opentelemetry/propagators/textmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
aabmass marked this conversation as resolved.
Show resolved Hide resolved
"""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
Expand All @@ -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:
Expand All @@ -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):
aabmass marked this conversation as resolved.
Show resolved Hide resolved
"""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.
Expand All @@ -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
"""
codeboten marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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.

Expand All @@ -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.

"""

Expand Down
Loading