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 14 commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
"""
This module serves as an example for baggage, which exists
to pass application-defined key-value pairs from service to service.
"""
# import opentelemetry.ext.http_requests
# from opentelemetry.ext.wsgi import OpenTelemetryMiddleware

import flask
from flask import request
import requests

from opentelemetry import propagation, trace
from opentelemetry.distributedcontext import CorrelationContextManager
from opentelemetry.sdk.context.propagation import b3_format
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace.export import (
BatchExportSpanProcessor,
ConsoleSpanExporter,
)
from opentelemetry.baggage import BaggageManager


def configure_opentelemetry(flask_app: flask.Flask):
Copy link
Member

Choose a reason for hiding this comment

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

this whole function looks good to me.

trace.set_preferred_tracer_implementation(lambda T: Tracer())

# Global initialization
(baggage_extractor, baggage_injector) = BaggageManager.http_propagator()
(b3_extractor, b3_injector) = b3_format.http_propagator()
# propagation.set_http_extractors([b3_extractor, baggage_extractor])
# propagation.set_http_injectors([b3_injector, baggage_injector])
propagation.set_http_extractors([b3_extractor])
propagation.set_http_injectors([b3_injector])

# opentelemetry.ext.http_requests.enable(trace.tracer())
# flask_app.wsgi_app = OpenTelemetryMiddleware(flask_app.wsgi_app)


def fetch_from_service_b() -> str:
# Inject the contexts to be propagated. Note that there is no direct
# reference to tracing or baggage.
headers = {"Accept": "application/json"}
propagation.inject(headers)
print(headers)
resp = requests.get("https://opentelemetry.io", headers=headers)
return resp.text


def fetch_from_service_c() -> str:
# Inject the contexts to be propagated. Note that there is no direct
# reference to tracing or baggage.
headers = {"Accept": "application/json"}
propagation.inject(headers)
print(headers)
resp = requests.get("https://opentelemetry.io", headers=headers)
return resp.text

codeboten marked this conversation as resolved.
Show resolved Hide resolved

app = flask.Flask(__name__)


@app.route("/")
def hello():
tracer = trace.tracer()
tracer.add_span_processor(BatchExportSpanProcessor(ConsoleSpanExporter()))
with propagation.extract(request.headers):
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this differs from the example you put in the description. Is there one that you preferred?

In my opinion, this method looks great, and reduces a lot of boilerplate in comparison with the other example posted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, this is the same as the "Simpler API" example for anyone that doesn't need to use the context explicitly. As I suspect this would be the more common case, I like how much simpler it is.

# extract a baggage header
with tracer.start_as_current_span("service-span"):
with tracer.start_as_current_span("external-req-span"):
headers = {"Accept": "application/json"}
propagation.inject(headers)
version = CorrelationContextManager.value("version")
if version == "2.0":
return fetch_from_service_c()

return fetch_from_service_b()


request_headers = {
"Accept": "application/json",
"x-b3-traceid": "038c3fb613811e30898424c863eeae5a",
"x-b3-spanid": "6c7f9e56212a6ffa",
"x-b3-sampled": "0",
}

if __name__ == "__main__":
configure_opentelemetry(app)
app.run(debug=True)
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
import requests

import opentelemetry.ext.http_requests
from opentelemetry import propagators, trace
from opentelemetry import propagation, trace
from opentelemetry.ext.flask import instrument_app
from opentelemetry.sdk.context.propagation.b3_format import B3Format
from opentelemetry.sdk.context.propagation.b3_format import (
B3Extractor,
B3Injector,
)
from opentelemetry.sdk.trace import Tracer


Expand Down Expand Up @@ -51,7 +54,8 @@ def configure_opentelemetry(flask_app: flask.Flask):
# carry this value).

# TBD: can remove once default TraceContext propagators are installed.
propagators.set_global_httptextformat(B3Format())
propagation.set_http_extractors([B3Extractor])
propagation.set_http_injectors([B3Injector])

# Integrations are the glue that binds the OpenTelemetry API
# and the frameworks and libraries that are used together, automatically
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from flask import request as flask_request

import opentelemetry.ext.wsgi as otel_wsgi
from opentelemetry import propagators, trace
from opentelemetry import propagation, trace
from opentelemetry.context import Context
from opentelemetry.util import time_ns

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -56,8 +57,8 @@ def _before_flask_request():
span_name = flask_request.endpoint or otel_wsgi.get_default_span_name(
environ
)
parent_span = propagators.extract(
otel_wsgi.get_header_from_environ, environ
parent_span = propagation.extract(
Context.current(), otel_wsgi.get_header_from_environ, environ,
)

tracer = trace.tracer()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

from requests.sessions import Session

from opentelemetry import propagators
from opentelemetry import propagation
from opentelemetry.context import Context
from opentelemetry.trace import SpanKind

Expand Down Expand Up @@ -74,7 +74,9 @@ def instrumented_request(self, method, url, *args, **kwargs):
# to access propagators.

headers = kwargs.setdefault("headers", {})
propagators.inject(tracer, type(headers).__setitem__, headers)
propagation.inject(
Context.current(), type(headers).__setitem__, headers,
)
result = wrapped(self, method, url, *args, **kwargs) # *** PROCEED

span.set_attribute("http.status_code", result.status_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,13 @@
from deprecated import deprecated

import opentelemetry.trace as trace_api
from opentelemetry import propagators
from opentelemetry import propagation
from opentelemetry.context import Context
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.trace.propagation.context import (
from_context,
with_span_context,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -657,24 +662,22 @@ def inject(self, span_context, format, carrier):
# TODO: Finish documentation.
# pylint: disable=redefined-builtin
# This implementation does not perform the injecting by itself but
# uses the configured propagators in opentelemetry.propagators.
# uses the configured propagation in opentelemetry.propagation.
# TODO: Support Format.BINARY once it is supported in
# opentelemetry-python.
if format not in self._supported_formats:
raise opentracing.UnsupportedFormatException

propagator = propagators.get_global_httptextformat()
propagator.inject(
span_context.unwrap(), type(carrier).__setitem__, carrier
)
ctx = with_span_context(Context.current(), span_context.unwrap())
propagation.inject(ctx, type(carrier).__setitem__, carrier)

def extract(self, format, carrier):
"""Implements the ``extract`` method from the base class."""

# TODO: Finish documentation.
# pylint: disable=redefined-builtin
# This implementation does not perform the extracing by itself but
# uses the configured propagators in opentelemetry.propagators.
# uses the configured propagation in opentelemetry.propagation.
# TODO: Support Format.BINARY once it is supported in
# opentelemetry-python.
if format not in self._supported_formats:
Expand All @@ -684,7 +687,10 @@ def get_as_list(dict_object, key):
value = dict_object.get(key)
return [value] if value is not None else []

propagator = propagators.get_global_httptextformat()
otel_context = propagator.extract(get_as_list, carrier)
# propagator = propagation.get_global_httptextformat()

otel_context = from_context(
propagation.extract(Context.current(), get_as_list, carrier)
)

return SpanContextShim(otel_context)
55 changes: 33 additions & 22 deletions ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
import opentracing

import opentelemetry.ext.opentracing_shim as opentracingshim
from opentelemetry import propagators, trace
from opentelemetry.context.propagation.httptextformat import HTTPTextFormat
from opentelemetry import propagation, trace
from opentelemetry.context.propagation.httptextformat import (
HTTPExtractor,
HTTPInjector,
)
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.sdk.trace import Tracer

Expand All @@ -42,15 +45,18 @@ def setUpClass(cls):
trace.set_preferred_tracer_implementation(lambda T: Tracer())

# Save current propagator to be restored on teardown.
cls._previous_propagator = propagators.get_global_httptextformat()
cls._previous_injectors = propagation.get_http_injectors()
cls._previous_extractors = propagation.get_http_extractors()

# Set mock propagator for testing.
propagators.set_global_httptextformat(MockHTTPTextFormat)
propagation.set_http_extractors([MockHTTPExtractor])
propagation.set_http_injectors([MockHTTPInjector])

@classmethod
def tearDownClass(cls):
# Restore previous propagator.
propagators.set_global_httptextformat(cls._previous_propagator)
propagation.set_http_extractors(cls._previous_extractors)
propagation.set_http_injectors(cls._previous_injectors)

def test_shim_type(self):
# Verify shim is an OpenTracing tracer.
Expand Down Expand Up @@ -474,8 +480,8 @@ def test_inject_http_headers(self):

headers = {}
self.shim.inject(context, opentracing.Format.HTTP_HEADERS, headers)
self.assertEqual(headers[MockHTTPTextFormat.TRACE_ID_KEY], str(1220))
self.assertEqual(headers[MockHTTPTextFormat.SPAN_ID_KEY], str(7478))
self.assertEqual(headers[_TRACE_ID_KEY], str(1220))
self.assertEqual(headers[_SPAN_ID_KEY], str(7478))

def test_inject_text_map(self):
"""Test `inject()` method for Format.TEXT_MAP."""
Expand All @@ -486,8 +492,8 @@ def test_inject_text_map(self):
# Verify Format.TEXT_MAP
text_map = {}
self.shim.inject(context, opentracing.Format.TEXT_MAP, text_map)
self.assertEqual(text_map[MockHTTPTextFormat.TRACE_ID_KEY], str(1220))
self.assertEqual(text_map[MockHTTPTextFormat.SPAN_ID_KEY], str(7478))
self.assertEqual(text_map[_TRACE_ID_KEY], str(1220))
self.assertEqual(text_map[_SPAN_ID_KEY], str(7478))

def test_inject_binary(self):
"""Test `inject()` method for Format.BINARY."""
Expand All @@ -503,8 +509,8 @@ def test_extract_http_headers(self):
"""Test `extract()` method for Format.HTTP_HEADERS."""

carrier = {
MockHTTPTextFormat.TRACE_ID_KEY: 1220,
MockHTTPTextFormat.SPAN_ID_KEY: 7478,
_TRACE_ID_KEY: 1220,
_SPAN_ID_KEY: 7478,
}

ctx = self.shim.extract(opentracing.Format.HTTP_HEADERS, carrier)
Expand All @@ -515,8 +521,8 @@ def test_extract_text_map(self):
"""Test `extract()` method for Format.TEXT_MAP."""

carrier = {
MockHTTPTextFormat.TRACE_ID_KEY: 1220,
MockHTTPTextFormat.SPAN_ID_KEY: 7478,
_TRACE_ID_KEY: 1220,
_SPAN_ID_KEY: 7478,
}

ctx = self.shim.extract(opentracing.Format.TEXT_MAP, carrier)
Expand All @@ -531,16 +537,17 @@ def test_extract_binary(self):
self.shim.extract(opentracing.Format.BINARY, bytearray())


class MockHTTPTextFormat(HTTPTextFormat):
"""Mock propagator for testing purposes."""
_TRACE_ID_KEY = "mock-traceid"
_SPAN_ID_KEY = "mock-spanid"

TRACE_ID_KEY = "mock-traceid"
SPAN_ID_KEY = "mock-spanid"

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

@classmethod
def extract(cls, get_from_carrier, carrier):
trace_id_list = get_from_carrier(carrier, cls.TRACE_ID_KEY)
span_id_list = get_from_carrier(carrier, cls.SPAN_ID_KEY)
def extract(cls, context, get_from_carrier, carrier):
trace_id_list = get_from_carrier(carrier, _TRACE_ID_KEY)
span_id_list = get_from_carrier(carrier, _SPAN_ID_KEY)

if not trace_id_list or not span_id_list:
return trace.INVALID_SPAN_CONTEXT
Expand All @@ -549,7 +556,11 @@ def extract(cls, get_from_carrier, carrier):
trace_id=int(trace_id_list[0]), span_id=int(span_id_list[0])
)


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

@classmethod
def inject(cls, context, set_in_carrier, carrier):
set_in_carrier(carrier, cls.TRACE_ID_KEY, str(context.trace_id))
set_in_carrier(carrier, cls.SPAN_ID_KEY, str(context.span_id))
set_in_carrier(carrier, _TRACE_ID_KEY, str(context.trace_id))
set_in_carrier(carrier, _SPAN_ID_KEY, str(context.span_id))
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import typing
import wsgiref.util as wsgiref_util

from opentelemetry import propagators, trace
from opentelemetry import propagation, trace
from opentelemetry.context import Context
from opentelemetry.ext.wsgi.version import __version__ # noqa
from opentelemetry.trace.status import Status, StatusCanonicalCode

Expand Down Expand Up @@ -183,7 +184,10 @@ def __call__(self, environ, start_response):
"""

tracer = trace.tracer()
parent_span = propagators.extract(get_header_from_environ, environ)
# TODO: fix the return value here, expect a context
parent_span = propagation.extract(
Context.current(), get_header_from_environ, environ
)
span_name = get_default_span_name(environ)

span = tracer.start_span(
Expand Down
Loading