Ceilometer

TranslationHook middleware can lose track of its thread local storage

Bug #1481244 reported by Chris Dent
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Aodh
Fix Released
High
Chris Dent
Aodh 1.0.0
Ceilometer
Fix Released
High
Chris Dent
Ceilometer 5.0.0 "liberty"

Bug Description

In both aodh and ceilometer its possible for the following error to arise. This happens when an earlier exception may need to be translated. So not only is this error happening it is masking the one that actually matters.

2015-08-03 20:54:47.958508 Traceback (most recent call last):
2015-08-03 20:54:47.958533 File "/usr/local/lib/python2.7/dist-packages/webob/dec.py", line 130, in __call__
2015-08-03 20:54:47.958640 resp = self.call_func(req, *args, **self.kwargs)
2015-08-03 20:54:47.958651 File "/usr/local/lib/python2.7/dist-packages/webob/dec.py", line 195, in call_func
2015-08-03 20:54:47.958666 return self.func(req, *args, **kwargs)
2015-08-03 20:54:47.958673 File "/usr/local/lib/python2.7/dist-packages/oslo_middleware/request_id.py", line 37, in __call__
2015-08-03 20:54:47.958723 response = req.get_response(self.application)
2015-08-03 20:54:47.958733 File "/usr/local/lib/python2.7/dist-packages/webob/request.py", line 1317, in send
2015-08-03 20:54:47.959070 application, catch_exc_info=False)
2015-08-03 20:54:47.959082 File "/usr/local/lib/python2.7/dist-packages/webob/request.py", line 1281, in call_application
2015-08-03 20:54:47.959098 app_iter = application(self.environ, start_response)
2015-08-03 20:54:47.959106 File "/usr/local/lib/python2.7/dist-packages/webob/dec.py", line 130, in __call__
2015-08-03 20:54:47.959117 resp = self.call_func(req, *args, **self.kwargs)
2015-08-03 20:54:47.959124 File "/usr/local/lib/python2.7/dist-packages/webob/dec.py", line 195, in call_func
2015-08-03 20:54:47.959134 return self.func(req, *args, **kwargs)
2015-08-03 20:54:47.959140 File "/usr/local/lib/python2.7/dist-packages/keystonemiddleware/auth_token/__init__.py", line 434, in __call__
2015-08-03 20:54:47.959335 response = req.get_response(self._app)
2015-08-03 20:54:47.959346 File "/usr/local/lib/python2.7/dist-packages/webob/request.py", line 1317, in send
2015-08-03 20:54:47.959367 application, catch_exc_info=False)
2015-08-03 20:54:47.959374 File "/usr/local/lib/python2.7/dist-packages/webob/request.py", line 1281, in call_application
2015-08-03 20:54:47.959384 app_iter = application(self.environ, start_response)
2015-08-03 20:54:47.959390 File "/usr/local/lib/python2.7/dist-packages/pecan/middleware/recursive.py", line 56, in __call__
2015-08-03 20:54:47.959453 return self.application(environ, start_response)
2015-08-03 20:54:47.959462 File "/opt/stack/new/aodh/aodh/api/middleware.py", line 90, in __call__
2015-08-03 20:54:47.959539 error = hook.local_error.translatable_error
2015-08-03 20:54:47.959554 AttributeError: 'thread._local' object has no attribute 'translatable_error'

Since this is a pecan based middleware it's not clear why thread specific storage is required, doesn't pecan provide a pseudo-global request object that can be latched on to?

Revision history for this message
Chris Dent (cdent) wrote :

This has become something of a common problem in the gate for getting aodh functional tests running: http://logs.openstack.org/40/207840/10/experimental/gate-aodh-dsvm-functional-mongodb/644081f/logs/apache/aodh.txt.gz

Julien Danjou (jdanjou)
Changed in aodh:
assignee: nobody → Julien Danjou (jdanjou)
status: New → Confirmed
Changed in ceilometer:
status: New → Confirmed
assignee: nobody → Julien Danjou (jdanjou)
importance: Undecided → High
Changed in aodh:
importance: Undecided → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to aodh (master)

Fix proposed to branch: master
Review: https://review.openstack.org/210065

Changed in aodh:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/210418

Changed in aodh:
assignee: Julien Danjou (jdanjou) → Chris Dent (chdent)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on aodh (master)

Change abandoned by Julien Danjou (<email address hidden>) on branch: master
Review: https://review.openstack.org/210065
Reason: Abandoned for Chris patch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ceilometer (master)

Fix proposed to branch: master
Review: https://review.openstack.org/210503

Changed in ceilometer:
assignee: Julien Danjou (jdanjou) → Chris Dent (chdent)
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to aodh (master)

Reviewed: https://review.openstack.org/210418
Committed: https://git.openstack.org/cgit/openstack/aodh/commit/?id=9e07e7d971299b4e37848657aa26edf6e5b76ac2
Submitter: Jenkins
Branch: master

commit 9e07e7d971299b4e37848657aa26edf6e5b76ac2
Author: Chris Dent <email address hidden>
Date: Fri Aug 7 13:26:23 2015 +0000

    Correct thread handling in TranslationHook

    TranslationHook was using a threading.local() to store an error
    message however the threading.local() was being created in __init__
    of the Hook. Hooks are not per request, so this wasn't really
    working as planned.

    Now, instead of using threading.local() at all, we just modify the
    environ held by pecan (and later used by the
    ParsableErrorMiddleware). environ is request-local.

    This change feels a bit crufty because it is but because of the way
    Exceptions are managed in the app there's not really any good way to do
    it. We could consider changing the way Exceptions related to error
    messages, but that would be a very large change.

    Change-Id: I463059df28f291cea0644b1a9908a907f146ba1f
    Closes-Bug: #1481244

Changed in aodh:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ceilometer (master)

Reviewed: https://review.openstack.org/210503
Committed: https://git.openstack.org/cgit/openstack/ceilometer/commit/?id=8290bdea6d23941d962b94ad5e0956732d04f43b
Submitter: Jenkins
Branch: master

commit 8290bdea6d23941d962b94ad5e0956732d04f43b
Author: Chris Dent <email address hidden>
Date: Fri Aug 7 15:45:00 2015 +0000

    Correct thread handling in TranslationHook

    TranslationHook was using a threading.local() to store an error
    message however the threading.local() was being created in __init__
    of the Hook. Hooks are not per request, so this wasn't really
    working as planned.

    Now, instead of using threading.local() at all, we just modify the
    environ held by pecan (and later used by the
    ParsableErrorMiddleware). environ is request-local.

    This change feels a bit crufty because it is but because of the way
    Exceptions are managed in the app there's not really any good way to do
    it. We could consider changing the way Exceptions related to error
    messages, but that would be a very large change.

    Duplicated from similar change in aodh:
    I463059df28f291cea0644b1a9908a907f146ba1f

    Change-Id: Ie561ec7ee7ec8b6d4a535ba7b20569b5a1101878
    Closes-Bug: #1481244

Changed in ceilometer:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in ceilometer:
milestone: none → liberty-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in aodh:
milestone: none → 1.0.0
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in ceilometer:
milestone: liberty-3 → 5.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.