Stop internal client txn_id bleeding into sharder logs

The LogAdapter txn_id is stored in a threading.local object that is a
class attribute, so all instances of the LogAdapter in same thread
will pick up the txn_id set in another instance. That means that after
the internal client has been used to make a request, all subsequent
sharder logs include the request txn_id.

The txn_id might be useful if an error occurs in _fetch_shard_ranges
but is annoying elsewhere.

Change-Id: I4e1961c13be301381907885579d4137cd0a9b16a

2 files changed, 50 insertions, 45 deletions

diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index 400794bee..41e93091b 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -309,7 +309,7 @@ class ContainerSharder(ContainerReplicator):
         internal_client_conf_path = conf.get('internal_client_conf_path',
                                              '/etc/swift/internal-client.conf')
         try:
-            self.swift = internal_client.InternalClient(
+            self.int_client = internal_client.InternalClient(
                 internal_client_conf_path,
                 'Swift Container Sharder',
                 request_tries,
@@ -458,7 +458,8 @@ class ContainerSharder(ContainerReplicator):
 
     def _fetch_shard_ranges(self, broker, newest=False, params=None,
                             include_deleted=False):
-        path = self.swift.make_path(broker.root_account, broker.root_container)
+        path = self.int_client.make_path(broker.root_account,
+                                         broker.root_container)
         params = params or {}
         params.setdefault('format', 'json')
         headers = {'X-Backend-Record-Type': 'shard',
@@ -467,30 +468,34 @@ class ContainerSharder(ContainerReplicator):
         if newest:
             headers['X-Newest'] = 'true'
         try:
-            resp = self.swift.make_request(
-                'GET', path, headers, acceptable_statuses=(2,), params=params)
-        except internal_client.UnexpectedResponse as err:
-            self.logger.warning("Failed to get shard ranges from %s: %s",
-                                broker.root_path, err)
-            return None
-        record_type = resp.headers.get('x-backend-record-type')
-        if record_type != 'shard':
-            err = 'unexpected record type %r' % record_type
-            self.logger.error("Failed to get shard ranges from %s: %s",
-                              broker.root_path, err)
-            return None
+            try:
+                resp = self.int_client.make_request(
+                    'GET', path, headers, acceptable_statuses=(2,),
+                    params=params)
+            except internal_client.UnexpectedResponse as err:
+                self.logger.warning("Failed to get shard ranges from %s: %s",
+                                    broker.root_path, err)
+                return None
+            record_type = resp.headers.get('x-backend-record-type')
+            if record_type != 'shard':
+                err = 'unexpected record type %r' % record_type
+                self.logger.error("Failed to get shard ranges from %s: %s",
+                                  broker.root_path, err)
+                return None
 
-        try:
-            data = json.loads(resp.body)
-            if not isinstance(data, list):
-                raise ValueError('not a list')
-            return [ShardRange.from_dict(shard_range)
-                    for shard_range in data]
-        except (ValueError, TypeError, KeyError) as err:
-            self.logger.error(
-                "Failed to get shard ranges from %s: invalid data: %r",
-                broker.root_path, err)
-        return None
+            try:
+                data = json.loads(resp.body)
+                if not isinstance(data, list):
+                    raise ValueError('not a list')
+                return [ShardRange.from_dict(shard_range)
+                        for shard_range in data]
+            except (ValueError, TypeError, KeyError) as err:
+                self.logger.error(
+                    "Failed to get shard ranges from %s: invalid data: %r",
+                    broker.root_path, err)
+            return None
+        finally:
+            self.logger.txn_id = None
 
     def _put_container(self, node, part, account, container, headers, body):
         try:
diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py
index 0fdcfd20c..9b9577cc8 100644
--- a/test/unit/container/test_sharder.py
+++ b/test/unit/container/test_sharder.py
@@ -620,7 +620,7 @@ class TestSharder(BaseTestSharder):
         exc = internal_client.UnexpectedResponse(
             'Unexpected response: 404', None)
         with self._mock_sharder() as sharder:
-            sharder.swift.make_request.side_effect = exc
+            sharder.int_client.make_request.side_effect = exc
             self.assertIsNone(sharder._fetch_shard_ranges(broker))
         lines = sharder.logger.get_lines_for_level('warning')
         self.assertIn('Unexpected response: 404', lines[0])
@@ -631,7 +631,7 @@ class TestSharder(BaseTestSharder):
             with self._mock_sharder() as sharder:
                 mock_make_request = mock.MagicMock(
                     return_value=mock.MagicMock(headers=mock_resp_headers))
-                sharder.swift.make_request = mock_make_request
+                sharder.int_client.make_request = mock_make_request
                 self.assertIsNone(sharder._fetch_shard_ranges(broker))
             lines = sharder.logger.get_lines_for_level('error')
             self.assertIn('unexpected record type', lines[0])
@@ -649,7 +649,7 @@ class TestSharder(BaseTestSharder):
                 mock_make_request = mock.MagicMock(
                     return_value=mock.MagicMock(headers=mock_resp_headers,
                                                 body=mock_resp_body))
-                sharder.swift.make_request = mock_make_request
+                sharder.int_client.make_request = mock_make_request
                 self.assertIsNone(sharder._fetch_shard_ranges(broker))
             lines = sharder.logger.get_lines_for_level('error')
             self.assertIn('invalid data', lines[0])
@@ -668,11 +668,11 @@ class TestSharder(BaseTestSharder):
                 mock_make_request = mock.MagicMock(
                     return_value=mock.MagicMock(headers=mock_resp_headers,
                                                 body=mock_resp_body))
-                sharder.swift.make_request = mock_make_request
+                sharder.int_client.make_request = mock_make_request
                 mock_make_path = mock.MagicMock(return_value='/v1/a/c')
-                sharder.swift.make_path = mock_make_path
+                sharder.int_client.make_path = mock_make_path
                 actual = sharder._fetch_shard_ranges(broker, params=params)
-            sharder.swift.make_path.assert_called_once_with('a', 'c')
+            sharder.int_client.make_path.assert_called_once_with('a', 'c')
             self.assertFalse(sharder.logger.get_lines_for_level('error'))
             return actual, mock_make_request
 
@@ -3445,20 +3445,20 @@ class TestSharder(BaseTestSharder):
         def call_audit_container(exc=None):
             with self._mock_sharder() as sharder:
                 sharder.logger = debug_logger()
-                with mock.patch.object(
-                        sharder, '_audit_root_container') as mocked:
-                    with mock.patch.object(sharder, 'swift') as mock_swift:
-                        mock_response = mock.MagicMock()
-                        mock_response.headers = {'x-backend-record-type':
-                                                 'shard'}
-                        mock_response.body = json.dumps(
-                            [dict(sr) for sr in shard_ranges])
-                        mock_swift.make_request.return_value = mock_response
-                        mock_swift.make_request.side_effect = exc
-                        mock_swift.make_path = (lambda a, c:
-                                                '/v1/%s/%s' % (a, c))
-                        sharder.reclaim_age = 0
-                        sharder._audit_container(broker)
+                with mock.patch.object(sharder, '_audit_root_container') \
+                        as mocked, mock.patch.object(
+                            sharder, 'int_client') as mock_swift:
+                    mock_response = mock.MagicMock()
+                    mock_response.headers = {'x-backend-record-type':
+                                             'shard'}
+                    mock_response.body = json.dumps(
+                        [dict(sr) for sr in shard_ranges])
+                    mock_swift.make_request.return_value = mock_response
+                    mock_swift.make_request.side_effect = exc
+                    mock_swift.make_path = (lambda a, c:
+                                            '/v1/%s/%s' % (a, c))
+                    sharder.reclaim_age = 0
+                    sharder._audit_container(broker)
             mocked.assert_not_called()
             return sharder, mock_swift