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

Introduce a generic context #566

Merged
merged 16 commits into from
Mar 21, 2019
Merged

Introduce a generic context #566

merged 16 commits into from
Mar 21, 2019

Conversation

reyang
Copy link
Contributor

@reyang reyang commented Mar 19, 2019

This is part of the #564 effort.

The RuntimeContext class provides a consistent way of manipulating/debugging context for both TLS and asyncio environment.

Please take a look at the examples first. I will start to add documents and test cases once we're okay on the general direction/naming/interface.

@c24t

@reyang reyang requested review from c24t, songy23 and a team as code owners March 19, 2019 22:31
@@ -0,0 +1,26 @@
from opencensus.common.runtime_context import RuntimeContext

RuntimeContext.register_slot('current_span', None)
Copy link
Contributor Author

@reyang reyang Mar 19, 2019

Choose a reason for hiding this comment

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

This is to demonstrate how we can leverage RuntimeContext to build trace/execution_context (tracer, current_span, attrs), tags/execution_context, W3C CorrelationContext and TraceContext. So all of them will benefit from the generic context in async and explicit threading (customer explicitly created threads and propagate the context manually) scenario.

In this way Dynatrace could also introduce an extension/exporter which registers its own context, without having to pollute span_context (e.g. if we want to propagate something within the process, but don't want to serialize/send across the wire).

Copy link
Member

Choose a reason for hiding this comment

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

Would you expect e.g. the correlation context package to have another single module-level context object? Like:

correlation_context = RuntimeContext.register_slot('correlation_context', dict)

This makes sense to me, but thinking about the API: as a user, why use RuntimeContext instead of contextvars.Context directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I will definitely try this approach.

Regarding letting user to use contextvars.Context, that is our direction - we don't want to introduce another context if the runtime already has it.

For now, given we need to support Python 2.7/3.4-3.6, probably we need to backport? I checked contextvars and didn't see a good way to backport as the logic was implemented as a native module (in C rather than Python code).

Choose a reason for hiding this comment

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

https://pypi.org/project/contextvars/ for 3.6 we have contextvars looks like.

Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

The direction and names look great to me.

Some questions about the future of this work: Should we have a common context API that we include in the spec, and that all language clients implement? If so, what does the API look like, and what do we need that isn't offered in existing implementations (contextvars, grpc.Context, go's context.Context, etc.)?

@@ -0,0 +1,26 @@
from opencensus.common.runtime_context import RuntimeContext

RuntimeContext.register_slot('current_span', None)
Copy link
Member

Choose a reason for hiding this comment

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

Would you expect e.g. the correlation context package to have another single module-level context object? Like:

correlation_context = RuntimeContext.register_slot('correlation_context', dict)

This makes sense to me, but thinking about the API: as a user, why use RuntimeContext instead of contextvars.Context directly?

contrib/opencensus-context/examples/py37.py Outdated Show resolved Hide resolved
contrib/opencensus-context/examples/py27.py Outdated Show resolved Hide resolved
@c24t
Copy link
Member

c24t commented Mar 20, 2019

cc @bogdandrutu, who championed using the gRPC context in java and may have opinions about the API.

Other reviewers: see the python contextvars docs and the PEP that introduced context variables.

We currently use threadlocals to store the execution context, which fails predictably with asyncio (and other event loop frameworks like tornado). Moving to contextvars will fix this.

@reyang reyang changed the base branch from experimental to master March 20, 2019 04:05
@reyang reyang changed the base branch from master to experimental March 20, 2019 04:06
@bogdandrutu
Copy link

contextvars are not available everywhere? Can we just use this?

@reyang
Copy link
Contributor Author

reyang commented Mar 20, 2019

contextvars are not available everywhere? Can we just use this?

contextvars was introduced in the latest version of Python (3.7). For old versions of Python, our best bet is Thread Local (and it doesn't seem that we can backport such feature, since contextvars was implemented as a native pyd module).

contrib/opencensus-context/examples/py37.py Outdated Show resolved Hide resolved
@@ -0,0 +1,26 @@
from opencensus.common.runtime_context import RuntimeContext

RuntimeContext.register_slot('current_span', None)

Choose a reason for hiding this comment

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

https://pypi.org/project/contextvars/ for 3.6 we have contextvars looks like.

@reyang reyang merged commit fbe4697 into experimental Mar 21, 2019
reyang added a commit that referenced this pull request Mar 23, 2019
Introduced the generic RuntimeContext.
reyang added a commit that referenced this pull request Mar 29, 2019
Introduced the generic RuntimeContext.
reyang added a commit that referenced this pull request Apr 1, 2019
Introduced the generic RuntimeContext.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants