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

Adding Context API #395

Merged
merged 62 commits into from
Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
1500bec
Context API
ocelotl Feb 3, 2020
3bdf1c8
Lint fixes
Feb 4, 2020
e144871
Merge remote-tracking branch 'origin/master' into adding-context-api
Feb 4, 2020
569878e
adding support for python 3.4 via thread-local
Feb 4, 2020
55c817a
Fix lint
Feb 4, 2020
746e591
review feedback
Feb 4, 2020
b53bdea
fix docs build
Feb 4, 2020
76a8c03
adding named tracer support back
Feb 4, 2020
3fe5a23
mypy fixes
Feb 4, 2020
1f8d67f
more mypy fixes
Feb 4, 2020
4bdd27c
lint fixes
Feb 4, 2020
b28fb3f
moving contextvars and threadlocal context to sdk
Feb 4, 2020
51af769
adding documentation
Feb 5, 2020
530c21a
documentation
Feb 5, 2020
16eb703
replace get_value w/ context.get_value
Feb 5, 2020
ddd81a5
log as error. more cleanup
Feb 5, 2020
b935bf5
zipkin exporter needs the sdk
Feb 5, 2020
15805a4
small cleanups
Feb 5, 2020
68b98e5
don't bother constructing a map for entry_points
Feb 5, 2020
3a2b0ef
cleaning up more code, adding some context tests
Feb 5, 2020
62c76ec
mypy fixes
Feb 5, 2020
5dca7cc
Fix ThreadlocalContext, use ContextVarsContext for tests with python …
Feb 5, 2020
7203e72
fix tests
Feb 5, 2020
e43f168
as per review feedback, removing use
Feb 5, 2020
f81a673
adding tests for sdk threadlocal and contextvar contexts
Feb 5, 2020
ba65845
removing create_key
Feb 6, 2020
ed187bf
add try/except
Feb 6, 2020
2b8c69f
adding tests for threads and asyncio
Feb 6, 2020
1a0217f
filling in asyncio test
Feb 6, 2020
212b14b
Test
ocelotl Feb 6, 2020
dbfeb98
Revert "Test"
ocelotl Feb 6, 2020
2235bc3
fix threadlocal behaviour, more feedback changes
Feb 6, 2020
5dc9fd5
fix context restore bug
Feb 7, 2020
02c7c4c
simplifying code, removing unnecessary methods
Feb 7, 2020
ae2cbc4
cleaning up docs
Feb 7, 2020
6f99621
Merge branch 'master' into adding-context-api
codeboten Feb 7, 2020
01e0054
Splitting the Context interface
Feb 7, 2020
ebcf0bd
update RuntimeContext when setting current
Feb 10, 2020
ddf3c8d
Merge remote-tracking branch 'origin/master' into adding-context-api
Feb 10, 2020
6f9780b
add set_current/get_current on RuntimeContext to store current Contex…
Feb 10, 2020
49b6abd
cleaning up mypy and fixing 3.4 tests
Feb 11, 2020
4c5083a
cleaning up tests to use context api interface
Feb 11, 2020
5a8a9fb
fix mypy
Feb 11, 2020
d5da10c
fix 3.4 tests
Feb 11, 2020
7995ac3
fixing docs
Feb 11, 2020
dbbdefb
Merge remote-tracking branch 'origin/master' into adding-context-api
Feb 11, 2020
37f1555
return the old context when calling set_current to allow for restore
Feb 11, 2020
81f06a9
mypy fixes
Feb 11, 2020
f29111a
removing snapshot from runtime context
Feb 11, 2020
057c22c
Merge remote-tracking branch 'origin/master' into adding-context-api
Feb 11, 2020
5376e60
cleaning up unused methods
Feb 11, 2020
e7d0286
lint fixes
Feb 11, 2020
0cee462
further sinmplifying RuntimeContext
Feb 11, 2020
f8032c1
lint fix
Feb 11, 2020
02b8f71
Update opentelemetry-api/src/opentelemetry/context/__init__.py
codeboten Feb 12, 2020
727ae50
adding test, updating copyright on new files
Feb 12, 2020
c9c66f3
Merge branch 'adding-context-api' of github.com:codeboten/opentelemet…
Feb 12, 2020
f25992f
Removing unnecessary copy
Feb 12, 2020
91d2ebc
set default for contextvars
Feb 12, 2020
335678e
feedback from review
Feb 13, 2020
619fc1b
minor fix
Feb 13, 2020
1f9bd76
review feedback
Feb 13, 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
@@ -1,7 +1,7 @@
opentelemetry.context.base\_context module
==========================================

.. automodule:: opentelemetry.context.base_context
.. automodule:: opentelemetry.context.context
:members:
:undoc-members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/opentelemetry.context.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Submodules

.. toctree::

opentelemetry.context.base_context
opentelemetry.context.context

Module contents
---------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@

from requests.sessions import Session

from opentelemetry import propagators
from opentelemetry.context import Context
from opentelemetry import context, propagators
from opentelemetry.ext.http_requests.version import __version__
from opentelemetry.trace import SpanKind

Expand Down Expand Up @@ -54,7 +53,7 @@ def enable(tracer_source):

@functools.wraps(wrapped)
def instrumented_request(self, method, url, *args, **kwargs):
if Context.suppress_instrumentation:
if context.get_value("suppress_instrumentation"):
return wrapped(self, method, url, *args, **kwargs)

# See
Expand Down
7 changes: 7 additions & 0 deletions opentelemetry-api/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,11 @@
"/tree/master/opentelemetry-api"
),
zip_safe=False,
entry_points={
"opentelemetry_context": [
"default_context = "
"opentelemetry.context.default_context:"
"DefaultContext",
]
},
)
256 changes: 118 additions & 138 deletions opentelemetry-api/src/opentelemetry/context/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,141 +12,121 @@
# See the License for the specific language governing permissions and
# limitations under the License.


"""
The OpenTelemetry context module provides abstraction layer on top of
thread-local storage and contextvars. The long term direction is to switch to
contextvars provided by the Python runtime library.

A global object ``Context`` is provided to access all the context related
functionalities::

>>> from opentelemetry.context import Context
>>> Context.foo = 1
>>> Context.foo = 2
>>> Context.foo
2

When explicit thread is used, a helper function
``Context.with_current_context`` can be used to carry the context across
threads::

from threading import Thread
from opentelemetry.context import Context

def work(name):
print('Entering worker:', Context)
Context.operation_id = name
print('Exiting worker:', Context)

if __name__ == '__main__':
print('Main thread:', Context)
Context.operation_id = 'main'

print('Main thread:', Context)

# by default context is not propagated to worker thread
thread = Thread(target=work, args=('foo',))
thread.start()
thread.join()

print('Main thread:', Context)

# user can propagate context explicitly
thread = Thread(
target=Context.with_current_context(work),
args=('bar',),
)
thread.start()
thread.join()

print('Main thread:', Context)

Here goes another example using thread pool::

import time
import threading

from multiprocessing.dummy import Pool as ThreadPool
from opentelemetry.context import Context

_console_lock = threading.Lock()

def println(msg):
with _console_lock:
print(msg)

def work(name):
println('Entering worker[{}]: {}'.format(name, Context))
Context.operation_id = name
time.sleep(0.01)
println('Exiting worker[{}]: {}'.format(name, Context))

if __name__ == "__main__":
println('Main thread: {}'.format(Context))
Context.operation_id = 'main'
pool = ThreadPool(2) # create a thread pool with 2 threads
pool.map(Context.with_current_context(work), [
'bear',
'cat',
'dog',
'horse',
'rabbit',
])
pool.close()
pool.join()
println('Main thread: {}'.format(Context))

Here goes a simple demo of how async could work in Python 3.7+::

import asyncio

from opentelemetry.context import Context

class Span(object):
def __init__(self, name):
self.name = name
self.parent = Context.current_span

def __repr__(self):
return ('{}(name={}, parent={})'
.format(
type(self).__name__,
self.name,
self.parent,
))

async def __aenter__(self):
Context.current_span = self

async def __aexit__(self, exc_type, exc, tb):
Context.current_span = self.parent

async def main():
print(Context)
async with Span('foo'):
print(Context)
await asyncio.sleep(0.1)
async with Span('bar'):
print(Context)
await asyncio.sleep(0.1)
print(Context)
await asyncio.sleep(0.1)
print(Context)

if __name__ == '__main__':
asyncio.run(main())
"""

from .base_context import BaseRuntimeContext

__all__ = ["Context"]

try:
from .async_context import AsyncRuntimeContext

Context = AsyncRuntimeContext() # type: BaseRuntimeContext
except ImportError:
from .thread_local_context import ThreadLocalRuntimeContext

Context = ThreadLocalRuntimeContext()
import logging
import typing
from os import environ

from pkg_resources import iter_entry_points

from opentelemetry.context.context import Context

logger = logging.getLogger(__name__)
_CONTEXT = None # type: typing.Optional[Context]


def _copy_context(context: typing.Optional[Context]) -> Context:
Copy link
Member

@Oberon00 Oberon00 Feb 5, 2020

Choose a reason for hiding this comment

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

I'd prefer writing these methods in the form

if context is None:
  context = get_current()
return some_action(context)

or even return some_action(context or get_current()).

EDIT: is (not) None is fine

if context is not None:
return context.copy()
return get_current().copy()


def create_key(key: str) -> "object":
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
# FIXME Implement this
raise NotImplementedError


def get_value(key: str, context: typing.Optional[Context] = None) -> "object":
"""
To access the local state of an concern, the Context API
c24t marked this conversation as resolved.
Show resolved Hide resolved
provides a function which takes a context and a key as input,
and returns a value.

Args:
key: The key of the value to retrieve.
context: The context from which to retrieve the value, if None, the current context is used.
"""
if context is not None:
return context.get_value(key)
return get_current().get_value(key)


def set_value(
mauriciovasquezbernal marked this conversation as resolved.
Show resolved Hide resolved
key: str, value: "object", context: typing.Optional[Context] = None
) -> Context:
"""
To record the local state of a cross-cutting concern, the
Context API provides a function which takes a context, a
key, and a value as input, and returns an updated context
which contains the new value.

Args:
key: The key of the entry to set
value: The value of the entry to set
context: The context to copy, if None, the current context is used
"""
new_context = _copy_context(context)
new_context.set_value(key, value)
return new_context


def remove_value(
key: str, context: typing.Optional[Context] = None
) -> Context:
"""
To remove a value, this method returns a new context with the key cleared.
Note that the removed value still remains present in the old context.

Args:
key: The key of the entry to remove
context: The context to copy, if None, the current context is used
"""
new_context = _copy_context(context)
new_context.remove_value(key)
return new_context


def get_current() -> Context:
"""
To access the context associated with program execution,
the Context API provides a function which takes no arguments
and returns a Context.
"""
global _CONTEXT # pylint: disable=global-statement
if _CONTEXT is None:
# FIXME use a better implementation of a configuration manager to avoid having
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
# to get configuration values straight from environment variables
_available_contexts = {} # typing.Dict[str, Context]

for entry_point in iter_entry_points("opentelemetry_context"):
try:
_available_contexts[entry_point.name] = entry_point.load() # type: ignore
except Exception: # pylint: disable=broad-except
logger.error("Could not load entry_point %s", entry_point.name)

configured_context = environ.get(
"OPENTELEMETRY_CONTEXT", "default_context"
) # type: str
_CONTEXT = _available_contexts[configured_context]() # type: ignore
return _CONTEXT # type: ignore


def set_current(context: Context) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

If this is supposed to represent the current context, then we cant just use a global variable. We need a context-local variable (or at least a thread local variable). But this is already what the context inside does, so it seems we have mixed up "current Context" and "configured context manager".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe my interpretation of what set_current is intended to achieve is wrong. My understanding is that set_current is used here to update the global context which can then be referenced other places in the application with updated values rather than updating the context within a context manager. This I think can be achieved with the global here, though I might be missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Oberon00 , what do you mean with "configured context manager"?

Copy link
Member

@Oberon00 Oberon00 Feb 6, 2020

Choose a reason for hiding this comment

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

I think we have to distinguish four things:

  1. A single context object, which is basically just a String -> Object dictionary. There is only at most one value for each key in a particular context. I think we are currently missing that concept.
  2. The current value of a particular context variable/key in some context (what get_value returns).
  3. The context manager, which is responsible for determining what the "current" context is, creating a new context object based on the parent context if a new logical execution context is entered / get_current is called in a logical execution context where no context object exists yet, etc.. This is what we currently call (IMHO confusingly) "Context".
  4. The logical execution context, an abstract concept, not a class or object. E.g. a thread of execution, the context of a async task, a greenlet, etc. The context manager defines what an execution context is and ensures that there is a 1:1 mapping from logical execution contexts to context objects.

EDIT: set_context should then set the context object in the current logical execution context.

EDIT2: As a concrete example, consider setting a value in a context object and then passing it (e.g. via a queue) to another, already running, thread. In that case, the other thread must be able to read that value back. This is not currently the case, as our "Context" is in fact a context manager and will return the return value of get_value(key, context) is potentially different when called from different logical execution contexts, even for identical context arguments.

EDIT3: I asked the spec folks: open-telemetry/opentelemetry-specification#424 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some clarifications:

  1. A single context object, which is basically just a String -> Object dictionary. There is only at most one value for each key in a particular context. I think we are currently missing that concept.

This is currently represented by the Context interface which is implemented by either ContextVarsContext or ThreadLocalContext depending on the version of Python

  1. The context manager, which is responsible for determining what the "current" context is, creating a new context object based on the parent context if a new logical execution context is entered / get_current is called in a logical execution context where no context object exists yet, etc.. This is what we currently call (IMHO confusingly) "Context".

This is implemented via the get_current & set_current methods

Copy link
Contributor

Choose a reason for hiding this comment

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

I was about to comment the same. Regarding point 1, here is the class that represents the concept of a single context object. It is implemented here, here and here.

Regarding your first EDIT, there is nothing named set_context. Did you mean set_current?

Copy link
Member

@Oberon00 Oberon00 Feb 7, 2020

Choose a reason for hiding this comment

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

@codeboten @ocelotl: No, ContextVarsContext and ThradLocalContext are not a contexts in the OTEP 66 sense. E.g. ThreadLocalContext is a mapping (thread, key) -> value instead of just key -> value. That's what I wanted to clarify in my EDIT2.

For example, in the OTEP, Propagator.extract returns a context. Which return type would you use given the types in this PR, so that you can give the context to another thread or execution context and it still keeps its values?

Copy link
Member

Choose a reason for hiding this comment

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

I think @Oberon00 has a good point here, and this will not work as expected.

Using his context queue example:

from queue import Queue
import asyncio

from opentelemetry import context
from opentelemetry.sdk.context.contextvars_context import ContextVarsContext

context.set_current(ContextVarsContext())

q = Queue()

async def main():
    q.put(context.get_current().copy())
    q.put(context.get_current().copy())
    asyncio.gather(work(1), work(2))

async def work(val):
    context.set_current(q.get_nowait())
    context.get_current().set_value('k', val)
    # At this point context.get_current() should be the one we popped from the
    # queue, and current_context.get_value('k') should be `val`

await main()

This works as expected, but only incidentally. If the call to set_current happens in one task in between the set_current and get_current calls in another, we get some weird behavior. Running effectively the same code, but manipulating the call order with an Event shows this:

from queue import Queue
from threading import Event
import asyncio

from opentelemetry import context
from opentelemetry.sdk.context.contextvars_context import ContextVarsContext

context.set_current(ContextVarsContext())

# Helper to print Contexts
def tos(context):
    return "<Context at {}: {}>".format(
        hex(id(context)),
        ','.join(["{}={}".format(k, v.get(None))
                  for k, v in context._contextvars.items()])
    )

q = Queue()
run1 = Event()
run2 = Event()

async def main():
    q.put(context.get_current().copy())
    q.put(context.get_current().copy())
    asyncio.gather(a1(), a2())

async def a1():
    # (Step 1)
    context.set_current(q.get_nowait())
    context.get_current().set_value('k', 1)

    # Expected context is current, k=1
    print("[task 1] {}".format(tos(context.get_current())))

    # (GOTO 2)
    run2.set(); await asyncio.sleep(0); run1.wait();

    # (Step 3)
    # Current context has changed AND k=None because the call to `set` k=2
    # happened in the other task!
    print("[task 1] {}".format(tos(context.get_current())))


async def a2():
    run2.wait()

    # (Step 2)
    context.set_current(q.get_nowait())
    context.get_current().set_value('k', 2)

    # Expected context is current, k=2
    print("[task 2] {}".format(tos(context.get_current())))

    # (GOTO 3)
    run1.set(); await asyncio.sleep(0);

await main()

This prints:

[task 1] <Context at 0x110349dd0: k=1>
[task 2] <Context at 0x110349250: k=2>
[task 1] <Context at 0x110349250: k=None>

Task 1 gets the context from the set_current call in task 2, but doesn't get its context variable values because set calls are scoped to the task! You could show roughly the same thing with threads and threadlocals.

(Sorry for the long example here)

Copy link
Member

Choose a reason for hiding this comment

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

@ocelotl Yes, sorry I meant set_current.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank for the clarifications. I've updated the Context class to be an immutable object. The portion of the Context class that was used to wrap threadlocal or contextvars implementation details is now RuntimeContext.

I still need to update the PR to not use a global _CONTEXT variable, but I think this change is at least a step in the right direction.

"""
To associate a context with program execution, the Context
API provides a function which takes a Context.

Args:
context: The context to use as current.
"""
global _CONTEXT # pylint: disable=global-statement
_CONTEXT = context


__all__ = [
"get_value",
"set_value",
"remove_value",
"get_current",
"set_current",
"Context",
]
45 changes: 0 additions & 45 deletions opentelemetry-api/src/opentelemetry/context/async_context.py

This file was deleted.

Loading