Use RequestContext store in oslo_context
Remove the _local store as we don't need it anymore. We should fetch the context directly from oslo_context's context module using the get_current() method. Use the fixture in oslo_context to adapt the test cases to work with the change outlined above. Since we are using RequestContext from oslo_context, adjust a bit of code to use resource_uuid in addition to instance_uuid as well. Added a fixture in the base test case itself to make sure that the context thread local store is cleared in between tests Change-Id: I09442d824400041768dbcddfbce932b175c767ab
This commit is contained in:
Davanum Srinivas
parent
6301575fef
commit
480cfc8514
|
|
@ -1,45 +0,0 @@
|
|||
# Copyright 2011 OpenStack Foundation.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# 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.
|
||||
|
||||
"""Local storage of variables using weak references"""
|
||||
|
||||
import threading
|
||||
import weakref
|
||||
|
||||
|
||||
class WeakLocal(threading.local):
|
||||
def __getattribute__(self, attr):
|
||||
rval = super(WeakLocal, self).__getattribute__(attr)
|
||||
if rval:
|
||||
# NOTE(mikal): this bit is confusing. What is stored is a weak
|
||||
# reference, not the value itself. We therefore need to lookup
|
||||
# the weak reference and return the inner value here.
|
||||
rval = rval()
|
||||
return rval
|
||||
|
||||
def __setattr__(self, attr, value):
|
||||
value = weakref.ref(value)
|
||||
return super(WeakLocal, self).__setattr__(attr, value)
|
||||
|
||||
|
||||
# NOTE(mikal): the name "store" should be deprecated in the future
|
||||
store = WeakLocal()
|
||||
|
||||
# A "weak" store uses weak references and allows an object to fall out of scope
|
||||
# when it falls out of scope in the code that uses the thread local storage. A
|
||||
# "strong" store will hold a reference to the object so that it never falls out
|
||||
# of scope.
|
||||
weak_store = WeakLocal()
|
||||
strong_store = threading.local()
|
||||
|
|
@ -21,7 +21,7 @@ import six
|
|||
from six import moves
|
||||
|
||||
from oslo.serialization import jsonutils
|
||||
from oslo_log import _local
|
||||
from oslo_context import context as context_utils
|
||||
|
||||
|
||||
def _dictify_context(context):
|
||||
|
|
@ -51,7 +51,7 @@ def _update_record_with_context(record):
|
|||
"""
|
||||
context = record.__dict__.get(
|
||||
'context',
|
||||
getattr(_local.store, 'context', None)
|
||||
context_utils.get_current()
|
||||
)
|
||||
d = _dictify_context(context)
|
||||
# Copy the context values directly onto the record so they can be
|
||||
|
|
@ -177,6 +177,11 @@ class ContextFormatter(logging.Formatter):
|
|||
# app-agnostic-logging-parameters blueprint.
|
||||
instance = getattr(context, 'instance', None)
|
||||
instance_uuid = getattr(context, 'instance_uuid', None)
|
||||
|
||||
# resource_uuid was introduced in oslo_context's
|
||||
# RequestContext
|
||||
resource_uuid = getattr(context, 'resource_uuid', None)
|
||||
|
||||
instance_extra = ''
|
||||
if instance:
|
||||
instance_extra = (self.conf.instance_format
|
||||
|
|
@ -184,6 +189,9 @@ class ContextFormatter(logging.Formatter):
|
|||
elif instance_uuid:
|
||||
instance_extra = (self.conf.instance_uuid_format
|
||||
% {'uuid': instance_uuid})
|
||||
elif resource_uuid:
|
||||
instance_extra = (self.conf.instance_uuid_format
|
||||
% {'uuid': resource_uuid})
|
||||
record.instance = instance_extra
|
||||
|
||||
# NOTE(sdague): default the fancier formatting params
|
||||
|
|
|
|||
|
|
@ -1,67 +0,0 @@
|
|||
# Copyright 2012 OpenStack Foundation.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# 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.
|
||||
|
||||
import threading
|
||||
|
||||
from oslotest import base as test_base
|
||||
from six import moves
|
||||
|
||||
from oslo_log import _local
|
||||
|
||||
|
||||
class Dict(dict):
|
||||
"""Make weak referencable object."""
|
||||
pass
|
||||
|
||||
|
||||
class LocalStoreTestCase(test_base.BaseTestCase):
|
||||
v1 = Dict(a='1')
|
||||
v2 = Dict(a='2')
|
||||
v3 = Dict(a='3')
|
||||
|
||||
def setUp(self):
|
||||
super(LocalStoreTestCase, self).setUp()
|
||||
# NOTE(mrodden): we need to make sure that local store
|
||||
# gets imported in the current python context we are
|
||||
# testing in (eventlet vs normal python threading) so
|
||||
# we test the correct type of local store for the current
|
||||
# threading model
|
||||
moves.reload_module(_local)
|
||||
|
||||
def test_thread_unique_storage(self):
|
||||
"""Make sure local store holds thread specific values."""
|
||||
expected_set = []
|
||||
_local.store.a = self.v1
|
||||
|
||||
def do_something():
|
||||
_local.store.a = self.v2
|
||||
expected_set.append(getattr(_local.store, 'a'))
|
||||
|
||||
def do_something2():
|
||||
_local.store.a = self.v3
|
||||
expected_set.append(getattr(_local.store, 'a'))
|
||||
|
||||
t1 = threading.Thread(target=do_something)
|
||||
t2 = threading.Thread(target=do_something2)
|
||||
t1.start()
|
||||
t2.start()
|
||||
t1.join()
|
||||
t2.join()
|
||||
|
||||
expected_set.append(getattr(_local.store, 'a'))
|
||||
|
||||
self.assertTrue(self.v1 in expected_set)
|
||||
self.assertTrue(self.v2 in expected_set)
|
||||
self.assertTrue(self.v3 in expected_set)
|
||||
|
|
@ -25,10 +25,10 @@ from oslo.config import fixture as fixture_config # noqa
|
|||
from oslo.i18n import fixture as fixture_trans
|
||||
from oslo.serialization import jsonutils
|
||||
from oslo_context import context
|
||||
from oslo_context import fixture as fixture_context
|
||||
from oslotest import base as test_base
|
||||
import six
|
||||
|
||||
from oslo_log import _local
|
||||
from oslo_log import _options
|
||||
from oslo_log import formatters
|
||||
from oslo_log import handlers
|
||||
|
|
@ -36,7 +36,7 @@ from oslo_log import log
|
|||
|
||||
|
||||
def _fake_context():
|
||||
return context.RequestContext(1, 1)
|
||||
return context.RequestContext(1, 1, overwrite=True)
|
||||
|
||||
|
||||
class CommonLoggerTestsMixIn(object):
|
||||
|
|
@ -105,6 +105,8 @@ class LoggerTestCase(CommonLoggerTestsMixIn, test_base.BaseTestCase):
|
|||
class BaseTestCase(test_base.BaseTestCase):
|
||||
def setUp(self):
|
||||
super(BaseTestCase, self).setUp()
|
||||
self.context_fixture = self.useFixture(
|
||||
fixture_context.ClearRequestContext())
|
||||
self.config_fixture = self.useFixture(
|
||||
fixture_config.Config(cfg.ConfigOpts()))
|
||||
self.config = self.config_fixture.config
|
||||
|
|
@ -313,43 +315,31 @@ class ContextFormatterTestCase(LogTestBase):
|
|||
|
||||
def test_context_is_taken_from_tls_variable(self):
|
||||
ctxt = _fake_context()
|
||||
_local.store.context = ctxt
|
||||
try:
|
||||
self.log.info("bar")
|
||||
expected = "HAS CONTEXT [%s]: bar\n" % ctxt.request_id
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
finally:
|
||||
del _local.store.context
|
||||
self.log.info("bar")
|
||||
expected = "HAS CONTEXT [%s]: bar\n" % ctxt.request_id
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
|
||||
def test_contextual_information_is_imparted_to_3rd_party_log_records(self):
|
||||
ctxt = _fake_context()
|
||||
_local.store.context = ctxt
|
||||
try:
|
||||
sa_log = logging.getLogger('sqlalchemy.engine')
|
||||
sa_log.setLevel(logging.INFO)
|
||||
sa_log.info('emulate logging within sqlalchemy')
|
||||
sa_log = logging.getLogger('sqlalchemy.engine')
|
||||
sa_log.setLevel(logging.INFO)
|
||||
sa_log.info('emulate logging within sqlalchemy')
|
||||
|
||||
expected = ("HAS CONTEXT [%s]: emulate logging within "
|
||||
"sqlalchemy\n" % ctxt.request_id)
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
finally:
|
||||
del _local.store.context
|
||||
expected = ("HAS CONTEXT [%s]: emulate logging within "
|
||||
"sqlalchemy\n" % ctxt.request_id)
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
|
||||
def test_message_logging_3rd_party_log_records(self):
|
||||
ctxt = _fake_context()
|
||||
_local.store.context = ctxt
|
||||
_local.store.context.request_id = six.text_type('99')
|
||||
try:
|
||||
sa_log = logging.getLogger('sqlalchemy.engine')
|
||||
sa_log.setLevel(logging.INFO)
|
||||
message = self.trans_fixture.lazy('test ' + six.unichr(128))
|
||||
sa_log.info(message)
|
||||
ctxt.request_id = six.text_type('99')
|
||||
sa_log = logging.getLogger('sqlalchemy.engine')
|
||||
sa_log.setLevel(logging.INFO)
|
||||
message = self.trans_fixture.lazy('test ' + six.unichr(128))
|
||||
sa_log.info(message)
|
||||
|
||||
expected = ("HAS CONTEXT [%s]: %s\n" % (ctxt.request_id,
|
||||
six.text_type(message)))
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
finally:
|
||||
del _local.store.context
|
||||
expected = ("HAS CONTEXT [%s]: %s\n" % (ctxt.request_id,
|
||||
six.text_type(message)))
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
|
||||
def test_debugging_log(self):
|
||||
self.log.debug("baz")
|
||||
|
|
@ -381,20 +371,16 @@ class ContextFormatterTestCase(LogTestBase):
|
|||
|
||||
def test_unicode_conversion_in_formatter(self):
|
||||
ctxt = _fake_context()
|
||||
_local.store.context = ctxt
|
||||
ctxt.request_id = six.text_type('99')
|
||||
try:
|
||||
no_adapt_log = logging.getLogger('no_adapt')
|
||||
no_adapt_log.setLevel(logging.INFO)
|
||||
message = "Exception is (%s)"
|
||||
ex = Exception(self.trans_fixture.lazy('test' + six.unichr(128)))
|
||||
no_adapt_log.info(message, ex)
|
||||
message = six.text_type(message) % ex
|
||||
expected = "HAS CONTEXT [%s]: %s\n" % (ctxt.request_id,
|
||||
message)
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
finally:
|
||||
del _local.store.context
|
||||
no_adapt_log = logging.getLogger('no_adapt')
|
||||
no_adapt_log.setLevel(logging.INFO)
|
||||
message = "Exception is (%s)"
|
||||
ex = Exception(self.trans_fixture.lazy('test' + six.unichr(128)))
|
||||
no_adapt_log.info(message, ex)
|
||||
message = six.text_type(message) % ex
|
||||
expected = "HAS CONTEXT [%s]: %s\n" % (ctxt.request_id,
|
||||
message)
|
||||
self.assertEqual(expected, self.stream.getvalue())
|
||||
|
||||
|
||||
class ExceptionLoggingTestCase(LogTestBase):
|
||||
|
|
@ -472,9 +458,9 @@ class FancyRecordTestCase(LogTestBase):
|
|||
|
||||
def test_instance_key_in_log_msg(self):
|
||||
ctxt = _fake_context()
|
||||
ctxt.instance_uuid = '1234'
|
||||
ctxt.resource_uuid = '1234'
|
||||
self._validate_keys(ctxt, ('[%s]: [instance: %s]' %
|
||||
(ctxt.request_id, ctxt.instance_uuid)))
|
||||
(ctxt.request_id, ctxt.resource_uuid)))
|
||||
|
||||
|
||||
class DomainTestCase(LogTestBase):
|
||||
|
|
@ -727,7 +713,7 @@ class KeywordArgumentAdapterTestCase(BaseTestCase):
|
|||
msg, kwargs = a.process(
|
||||
'message', {'context': 'some context object',
|
||||
'instance': 'instance identifier',
|
||||
'instance_uuid': 'UUID for instance',
|
||||
'resource_uuid': 'UUID for instance',
|
||||
'anything': 'goes'}
|
||||
)
|
||||
self.assertEqual(
|
||||
|
|
@ -735,9 +721,9 @@ class KeywordArgumentAdapterTestCase(BaseTestCase):
|
|||
{'extra': {'anything': 'goes',
|
||||
'context': 'some context object',
|
||||
'extra_keys': ['anything', 'context',
|
||||
'instance', 'instance_uuid'],
|
||||
'instance', 'resource_uuid'],
|
||||
'instance': 'instance identifier',
|
||||
'instance_uuid': 'UUID for instance',
|
||||
'resource_uuid': 'UUID for instance',
|
||||
'anything': 'goes'}},
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue