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

Refactor execution_context to use RuntimeContext #573

Merged
merged 3 commits into from
Mar 22, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
61 changes: 24 additions & 37 deletions opencensus/trace/execution_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,81 +12,68 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import threading

from opencensus.common.runtime_context import RuntimeContext
from opencensus.trace.tracers import noop_tracer

_thread_local = threading.local()
_attrs_slot = RuntimeContext.register_slot('attrs', lambda: {})
_current_span_slot = RuntimeContext.register_slot('current_span', None)
_tracer_slot = RuntimeContext.register_slot('tracer', noop_tracer.NoopTracer())


def get_opencensus_tracer():
"""Get the opencensus tracer from thread local."""
return getattr(_thread_local, 'tracer', noop_tracer.NoopTracer())
"""Get the opencensus tracer from runtime context."""
return RuntimeContext.tracer


def set_opencensus_tracer(tracer):
"""Add the tracer to thread local."""
setattr(_thread_local, 'tracer', tracer)
"""Add the tracer to runtime context."""
RuntimeContext.tracer = tracer


def set_opencensus_attr(attr_key, attr_value):
# If there is no attrs, initialize it to empty dict.
attrs = getattr(_thread_local, 'attrs', {})

attrs = RuntimeContext.attrs.copy()
Copy link
Member

Choose a reason for hiding this comment

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

Setting attrs on the context directly instead of the current span might be unique to the python client. It looks like this is used to:

  1. store the host blacklist in the flask and django integrations
  2. store the current span ID in the httplib integration
  3. store the request object in the django integration

(1) seems like it could be static config instead, (2) seems like it could use the current span, and (3) seems to be used to get the trace headers from the request object, which we might be able to do at request time instead.

Hopefully we can remove this too, and only set attrs on the current context.

attrs[attr_key] = attr_value

setattr(_thread_local, 'attrs', attrs)
RuntimeContext.attrs = attrs


def set_opencensus_attrs(attrs):
setattr(_thread_local, 'attrs', attrs)
RuntimeContext.attrs = attrs


def get_opencensus_attr(attr_key):
attrs = getattr(_thread_local, 'attrs', None)

if attrs is not None:
return attrs.get(attr_key)

return None
return RuntimeContext.attrs.get(attr_key)


def get_opencensus_attrs():
return getattr(_thread_local, 'attrs', None)
return RuntimeContext.attrs


def get_current_span():
return getattr(_thread_local, 'current_span', None)
return RuntimeContext.current_span


def set_current_span(current_span):
setattr(_thread_local, 'current_span', current_span)
RuntimeContext.current_span = current_span


def get_opencensus_full_context():
_tracer = get_opencensus_tracer()
_span = get_current_span()
_attrs = get_opencensus_attrs()
return _tracer, _span, _attrs
attrs = RuntimeContext.attrs
current_span = RuntimeContext.current_span
tracer = RuntimeContext.tracer
return tracer, current_span, attrs


def set_opencensus_full_context(tracer, span, attrs):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This set_opencensus_full_context probably should be removed (in another PR):

  1. It is not the "full context" as it doesn't cover tags.
  2. RuntimeContext.apply provides the intended functionality, which can be used by propagating context to explicit threads.

set_opencensus_tracer(tracer)
set_current_span(span)
if not attrs:
set_opencensus_attrs({})
else:
set_opencensus_attrs(attrs)
set_opencensus_attrs(attrs or {})


def clean():
setattr(_thread_local, 'attrs', {})
if hasattr(_thread_local, 'current_span'):
delattr(_thread_local, 'current_span')
if hasattr(_thread_local, 'tracer'):
delattr(_thread_local, 'tracer')
_attrs_slot.clear()
_current_span_slot.clear()
_tracer_slot.clear()


def clear():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@c24t I think having a test-only method exposed publicly is evil. I plan to remove it or make it "private". Please let me know your thoughts.

BTW, _thread_local.__dict__.clear() will remove Thread-local storage introduced by other components (e.g. stats), not sure if this is intended. @liyanhui1228 might have some background on this?

Copy link
Member

Choose a reason for hiding this comment

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

I agree this is a cardinal sin. We may be able to remove this entirely soon -- if we stop storing the tracer and measure to view map in the context and control span and tag lifetimes with context managers then hopefully tests won't pollute the context (and should be able to run in parallel!).

"""Clear the thread local, used in test."""
_thread_local.__dict__.clear()
clean()
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
include_package_data=True,
long_description=open('README.rst').read(),
install_requires=[
'opencensus-context == 0.2.dev0',
'google-api-core >= 1.0.0, < 2.0.0',
],
extras_require={},
Expand Down