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

[WIP] Context Prop #325

Closed
wants to merge 73 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
5f84c2b
stopgap PR to start the discussion on context propagation.
toumorokoshi Nov 7, 2019
6bea6ec
Merge remote-tracking branch 'origin/master' into context-prop
Dec 10, 2019
bbb583d
adding Baggage API
Dec 10, 2019
c3f408b
no-op implementations of baggage injector/extractor
Dec 10, 2019
8ec6106
cleaning up baggage prop interface
Dec 11, 2019
9788f69
make extract/inject static
Dec 11, 2019
6a46a25
rename DistributedContext to CorrelationContext
Dec 11, 2019
5ef40df
adding ContextKeys for trace and distributedcontext
Dec 11, 2019
0442f29
moving api/propagators -> api/propagation. adding more to context api
Dec 11, 2019
622d787
add from_context/with_span_context helpers
Dec 12, 2019
317e937
splitting propagator into extractor/injector
Dec 12, 2019
64cfe9b
checkpoint checkin, this PR will not leave draft mode
Dec 13, 2019
22a5c64
implement context scoping
Dec 13, 2019
375b78e
minor cleanup
Dec 13, 2019
5ca5328
clean up example code
Dec 16, 2019
b475933
pass context where needed
Dec 16, 2019
c4829c4
removing baggage module
Dec 16, 2019
c7610fa
small lint improvements
Dec 16, 2019
7489eb5
more lint fixes
Dec 16, 2019
a4c4150
fixing a few more lint issues
Dec 17, 2019
0946846
fixing inject/extract signatures and tracecontext tests
Dec 17, 2019
033e27e
fixing tests
Dec 17, 2019
cc813bb
rename current -> snapshot
Dec 17, 2019
7391372
fix remaining sdk tests
Dec 17, 2019
fa6d437
small context cleanup, return obj, remove unused __getattr__
Dec 17, 2019
5fa8e02
store both current span and extracted span context
Dec 17, 2019
164ef68
test cleanup
Dec 17, 2019
b37c3b0
fix ot shim tests
Dec 17, 2019
4ddac90
fixing http_requests tests
Dec 18, 2019
f500132
refactoring common getter/setter for propagation
Dec 18, 2019
93e88de
add convenience methods to set/get span from context
Dec 18, 2019
72c0dbd
fix wsgi and flask tests
Dec 18, 2019
f3c8076
add parameter to extract to support custom getters
Dec 18, 2019
d82e4c5
changing to copy for now, still need better mechanism here
Dec 18, 2019
8927455
enable wsgi/request in example
Dec 18, 2019
a4b7d0c
fix tracecontext tests
Dec 18, 2019
3b8bf5d
move distributedcontext to correlationcontext
Dec 18, 2019
9efb6e4
lint fixes
Dec 18, 2019
5272bed
add default injector/extractor. move tracecontext propagator to sdk
Dec 18, 2019
ea0905c
use default propagator to avoid installing sdk
Dec 18, 2019
4c2c4de
moving context implementation to sdk
Dec 18, 2019
1447d7f
tests fixed after context move
Dec 19, 2019
1cd03c5
lint fixes
Dec 19, 2019
7c9597c
Revert "lint fixes"
Dec 19, 2019
4ca46d9
Revert "tests fixed after context move"
Dec 19, 2019
48c2a7d
Revert "moving context implementation to sdk"
Dec 19, 2019
c7130a1
lint fix
Dec 19, 2019
5169723
fix tracecontext tests
Dec 19, 2019
673224c
mypy cleanup
Dec 19, 2019
4f008a4
Merge 'origin/master' into context-prop
Dec 19, 2019
66d67b8
fix context prop example
Dec 20, 2019
4442a04
mypy cleanup. adding some ignores for now, will review later
Dec 20, 2019
17bb4b1
adding context merge method
Dec 20, 2019
b82dc78
adding documentation
Jan 8, 2020
b850f99
Adding tests for concurrency, fixing context
Jan 9, 2020
33a5b78
Clean up tests and context
Jan 13, 2020
be91061
rename from_context to span_context_from_context
Jan 13, 2020
f84c4d3
Fix example
Jan 13, 2020
b1ba228
Context cleanup
Jan 14, 2020
c3906ea
Revert behaviour of no context to INVALID_SPAN_CONTEXT
Jan 15, 2020
8d0b142
More refactors of Context
Jan 15, 2020
6965c33
Removing context correlation propagators
Jan 20, 2020
c129e82
Lint changes
Jan 20, 2020
0a6c385
Adding test and fixing restore behaviour
Jan 20, 2020
57ad2d2
adding restore test
Jan 20, 2020
cfdfc62
Pass context to set_value
Jan 20, 2020
257627c
Rename HTTPInjector HTTPExtractor to Injector Extractor
Jan 20, 2020
b210df8
Moving propagation api into propagation/__init__.py
Jan 20, 2020
73dbfab
Fix example
Jan 20, 2020
b9be7bd
Context refactor
Jan 21, 2020
d70a47c
Adding set_in_carrier parameter, updating docs
Jan 21, 2020
1e3ee56
rename dctx_api
Jan 21, 2020
47f521f
Gutting Context class
Jan 23, 2020
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
Prev Previous commit
Next Next commit
Rename HTTPInjector HTTPExtractor to Injector Extractor
Signed-off-by: Alex Boten <aboten@lightstep.com>
  • Loading branch information
Alex Boten committed Jan 20, 2020
commit 257627cfc3e941f3f6c40f995bf5ea56409477a7
8 changes: 4 additions & 4 deletions ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
from opentelemetry import propagation, trace
from opentelemetry.context.propagation import get_as_list, set_in_dict
from opentelemetry.context.propagation.httptextformat import (
HTTPExtractor,
HTTPInjector,
Extractor,
Injector,
)
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.sdk.trace import TracerSource
Expand Down Expand Up @@ -547,7 +547,7 @@ def test_extract_binary(self):
_SPAN_ID_KEY = "mock-spanid"


class MockHTTPExtractor(HTTPExtractor):
class MockHTTPExtractor(Extractor):
"""Mock extractor for testing purposes."""

@classmethod
Expand All @@ -565,7 +565,7 @@ def extract(cls, carrier, context=None, get_from_carrier=get_as_list):
)


class MockHTTPInjector(HTTPInjector):
class MockHTTPInjector(Injector):
"""Mock injector for testing purposes."""

@classmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,14 @@
from .binaryformat import BinaryFormat
from .httptextformat import (
ContextT,
DefaultHTTPExtractor,
DefaultHTTPInjector,
DefaultExtractor,
DefaultInjector,
Extractor,
Getter,
HTTPExtractor,
HTTPInjector,
Injector,
Setter,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the goal here to change these classes' import paths?

For example, I see opentelemetry-api/src/opentelemetry/propagation/__init__.py does:

from opentelemetry.context.propagation import DefaultHTTPExtractor

instead of

from opentelemetry.context.propagation.httptextformat import DefaultHTTPExtractor

Why do this?


__all__ = [
"BinaryFormat",
"ContextT",
"Getter",
"DefaultHTTPExtractor",
"DefaultHTTPInjector",
"HTTPExtractor",
"HTTPInjector",
"Setter",
]


def get_as_list(
dict_object: typing.Dict[str, str], key: str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
Getter = typing.Callable[[ContextT, str], typing.List[str]]


class HTTPExtractor(abc.ABC):
class Extractor(abc.ABC):
"""API for propagation of span context via headers.

TODO: update docs to reflect split into extractor/injector
Expand All @@ -38,7 +38,7 @@ class HTTPExtractor(abc.ABC):

import flask
import requests
from opentelemetry.context.propagation import HTTPExtractor
from opentelemetry.context.propagation import Extractor

PROPAGATOR = HTTPTextFormat()

Expand Down Expand Up @@ -80,27 +80,27 @@ def extract(
context: typing.Optional[Context] = None,
get_from_carrier: typing.Optional[Getter[ContextT]] = None,
) -> Context:
"""Create a SpanContext from values in the carrier.
"""Create a Context from values in the carrier.

The extract function should retrieve values from the carrier
object using get_from_carrier, and use values to populate a
SpanContext value and return it.
object using get_from_carrier, use values to populate a
Context value and return it.

Args:
get_from_carrier: a function that can retrieve zero
or more values from the carrier. In the case that
the value does not exist, return an empty list.
carrier: and object which contains values that are
used to construct a SpanContext. This object
used to construct a Context. This object
must be paired with an appropriate get_from_carrier
which understands how to extract a value from it.
context: The Context to read values from.
get_from_carrier: a function that can retrieve zero
or more values from the carrier. In the case that
the value does not exist, return an empty list.
Returns:
A SpanContext with configuration found in the carrier.

A Context with configuration found in the carrier.
"""


class HTTPInjector(abc.ABC):
class Injector(abc.ABC):
@classmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make these classmethods? Is it an error to ever instantiate an injector/extractor?

@abc.abstractmethod
def inject(
Expand All @@ -109,71 +109,27 @@ def inject(
context: typing.Optional[Context] = None,
set_in_carrier: typing.Optional[Setter[ContextT]] = None,
) -> None:
"""Inject values from a SpanContext into a carrier.
"""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
carrier.

Args:
context: The SpanContext to read values from.
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
know how to set header values on the carrier.

context: The Context to read values from.
set_in_carrier: A setter function that can set values
on the carrier.
"""


class DefaultHTTPExtractor(HTTPExtractor):
"""API for propagation of span context via headers.

TODO: update docs to reflect split into extractor/injector

This class provides an interface that enables extracting and injecting
span context into headers of HTTP requests. HTTP frameworks and clients
can integrate with HTTPTextFormat by providing the object containing the
headers, and a getter and setter function for the extraction and
injection of values, respectively.

Example::

import flask
import requests
from opentelemetry.context.propagation import HTTPExtractor

PROPAGATOR = HTTPTextFormat()



def get_header_from_flask_request(request, key):
return request.headers.get_all(key)

def set_header_into_requests_request(request: requests.Request,
key: str, value: str):
request.headers[key] = value

def example_route():
span_context = PROPAGATOR.extract(
get_header_from_flask_request,
flask.request
)
request_to_downstream = requests.Request(
"GET", "http://httpbin.org/get"
)
PROPAGATOR.inject(
span_context,
set_header_into_requests_request,
request_to_downstream
)
session = requests.Session()
session.send(request_to_downstream.prepare())

class DefaultExtractor(Extractor):
"""The default Extractor that is used when no Extractor implementation is configured.

.. _Propagation API Specification:
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-propagators.md
All operations are no-ops.
"""

@classmethod
Expand All @@ -183,51 +139,22 @@ def extract(
context: typing.Optional[Context] = None,
get_from_carrier: typing.Optional[Getter[ContextT]] = None,
) -> Context:
"""Create a SpanContext from values in the carrier.

The extract function should retrieve values from the carrier
object using get_from_carrier, and use values to populate a
SpanContext value and return it.

Args:
get_from_carrier: a function that can retrieve zero
or more values from the carrier. In the case that
the value does not exist, return an empty list.
carrier: and object which contains values that are
used to construct a SpanContext. This object
must be paired with an appropriate get_from_carrier
which understands how to extract a value from it.
Returns:
A SpanContext with configuration found in the carrier.

"""
if context:
return context
return Context.current()


class DefaultHTTPInjector(HTTPInjector):
class DefaultInjector(Injector):
"""The default Injector that is used when no Injector implementation is configured.

All operations are no-ops.
"""

@classmethod
def inject(
cls,
carrier: ContextT,
context: typing.Optional[Context] = None,
set_in_carrier: typing.Optional[Setter[ContextT]] = None,
) -> None:
"""Inject values from a SpanContext 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
carrier.

Args:
context: The SpanContext to read values from.
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
know how to set header values on the carrier.

"""
return None
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from typing import Optional

from opentelemetry.context import Context
from opentelemetry.context.propagation import HTTPExtractor, HTTPInjector
from opentelemetry.context.propagation import Extractor, Injector

PRINTABLE = frozenset(
itertools.chain(
Expand Down
30 changes: 13 additions & 17 deletions opentelemetry-api/src/opentelemetry/propagation/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
from opentelemetry.context import Context
from opentelemetry.context.propagation import (
ContextT,
DefaultHTTPExtractor,
DefaultHTTPInjector,
DefaultExtractor,
DefaultInjector,
Getter,
Setter,
)
Expand All @@ -29,14 +29,12 @@
def extract(
carrier: ContextT,
context: typing.Optional[Context] = None,
extractors: typing.Optional[
typing.List[httptextformat.HTTPExtractor]
] = None,
extractors: typing.Optional[typing.List[httptextformat.Extractor]] = None,
get_from_carrier: typing.Optional[Getter[ContextT]] = None,
) -> typing.Optional[Context]:
"""Load the parent SpanContext from values in the carrier.

Using the specified HTTPExtractor, the propagator will
Using the specified Extractor, the propagator will
extract a SpanContext from the carrier. If one is found,
it will be set as the parent context of the current span.

Expand Down Expand Up @@ -69,9 +67,7 @@ def extract(

def inject(
carrier: ContextT,
injectors: typing.Optional[
typing.List[httptextformat.HTTPInjector]
] = None,
injectors: typing.Optional[typing.List[httptextformat.Injector]] = None,
context: typing.Optional[Context] = None,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is set_in_carrier missing in this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Good catch, thanks!

) -> None:
"""Inject values from the current context into the carrier.
Expand All @@ -98,16 +94,16 @@ def inject(


_HTTP_TEXT_INJECTORS = [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to use a list instead of a tuple?

DefaultHTTPInjector
] # typing.List[httptextformat.HTTPInjector]
DefaultInjector
] # typing.List[httptextformat.Injector]

_HTTP_TEXT_EXTRACTORS = [
DefaultHTTPExtractor
] # typing.List[httptextformat.HTTPExtractor]
DefaultExtractor
] # typing.List[httptextformat.Extractor]


def set_http_extractors(
extractor_list: typing.List[httptextformat.HTTPExtractor],
extractor_list: typing.List[httptextformat.Extractor],
) -> None:
"""
To update the global extractor, the Propagation API provides a
Expand All @@ -118,7 +114,7 @@ def set_http_extractors(


def set_http_injectors(
injector_list: typing.List[httptextformat.HTTPInjector],
injector_list: typing.List[httptextformat.Injector],
) -> None:
"""
To update the global injector, the Propagation API provides a
Expand All @@ -128,15 +124,15 @@ def set_http_injectors(
_HTTP_TEXT_INJECTORS = injector_list # type: ignore


def get_http_extractors() -> typing.List[httptextformat.HTTPExtractor]:
def get_http_extractors() -> typing.List[httptextformat.Extractor]:
"""
To access the global extractor, the Propagation API provides
a function which returns an extractor.
"""
return _HTTP_TEXT_EXTRACTORS # type: ignore


def get_http_injectors() -> typing.List[httptextformat.HTTPInjector]:
def get_http_injectors() -> typing.List[httptextformat.Injector]:
"""
To access the global injector, the Propagation API provides a
function which returns an injector.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@

import opentelemetry.trace as trace
from opentelemetry.context.propagation import (
Extractor,
Getter,
HTTPExtractor,
HTTPInjector,
Injector,
Setter,
get_as_list,
set_in_dict,
Expand All @@ -36,12 +36,12 @@
SAMPLED_KEY = "x-b3-sampled"


def http_propagator() -> typing.Tuple[HTTPExtractor, HTTPInjector]:
def http_propagator() -> typing.Tuple[Extractor, Injector]:
""" TODO """
return B3Extractor, B3Injector


class B3Extractor(HTTPExtractor):
class B3Extractor(Extractor):
"""Propagator for the B3 HTTP header format.

See: https://github.com/openzipkin/b3-propagation
Expand Down Expand Up @@ -123,7 +123,7 @@ def extract(
)


class B3Injector(HTTPInjector):
class B3Injector(Injector):
@classmethod
def inject(
cls,
Expand Down
Loading