From a165d0ae51bf3d99caa136ba07ec922cc3159db5 Mon Sep 17 00:00:00 2001 From: Winson Chan Date: Thu, 26 May 2016 22:38:04 +0000 Subject: [PATCH] Add osprofiler option to trace operations Add --profile option, initialize osprofiler, and pass appropriate headers to the mistral server. Change-Id: Ib10a126902c707bd82c3fadff94e119fb18cf096 Implements: blueprint mistral-osprofiler --- mistralclient/api/client.py | 6 ++- mistralclient/api/httpclient.py | 5 +++ mistralclient/api/v2/client.py | 8 +++- mistralclient/shell.py | 36 +++++++++++++++++- mistralclient/tests/unit/test_client.py | 38 +++++++++++++++++++ mistralclient/tests/unit/test_httpclient.py | 42 +++++++++++++++++++++ mistralclient/tests/unit/test_shell.py | 7 ++++ requirements.txt | 1 + 8 files changed, 139 insertions(+), 4 deletions(-) diff --git a/mistralclient/api/client.py b/mistralclient/api/client.py index 40083578..a481dca8 100644 --- a/mistralclient/api/client.py +++ b/mistralclient/api/client.py @@ -20,7 +20,8 @@ from mistralclient.api.v2 import client as client_v2 def client(mistral_url=None, username=None, api_key=None, project_name=None, auth_url=None, project_id=None, endpoint_type='publicURL', service_type='workflow', - auth_token=None, user_id=None, cacert=None, insecure=False): + auth_token=None, user_id=None, cacert=None, insecure=False, + profile=None): if mistral_url and not isinstance(mistral_url, six.string_types): raise RuntimeError('Mistral url should be a string.') @@ -37,7 +38,8 @@ def client(mistral_url=None, username=None, api_key=None, auth_token=auth_token, user_id=user_id, cacert=cacert, - insecure=insecure + insecure=insecure, + profile=profile ) diff --git a/mistralclient/api/httpclient.py b/mistralclient/api/httpclient.py index 6dd59565..ed90f7d1 100644 --- a/mistralclient/api/httpclient.py +++ b/mistralclient/api/httpclient.py @@ -20,6 +20,8 @@ import requests import logging +import osprofiler.web + LOG = logging.getLogger(__name__) @@ -106,4 +108,7 @@ class HTTPClient(object): if user_id: headers['X-User-Id'] = user_id + # Add headers for osprofiler. + headers.update(osprofiler.web.get_trace_id_headers()) + return headers diff --git a/mistralclient/api/v2/client.py b/mistralclient/api/v2/client.py index 018876e8..c0a93395 100644 --- a/mistralclient/api/v2/client.py +++ b/mistralclient/api/v2/client.py @@ -15,6 +15,8 @@ import six +import osprofiler.profiler + from mistralclient.api import httpclient from mistralclient.api.v2 import action_executions from mistralclient.api.v2 import actions @@ -32,7 +34,8 @@ class Client(object): def __init__(self, mistral_url=None, username=None, api_key=None, project_name=None, auth_url=None, project_id=None, endpoint_type='publicURL', service_type='workflowv2', - auth_token=None, user_id=None, cacert=None, insecure=False): + auth_token=None, user_id=None, cacert=None, insecure=False, + profile=None): if mistral_url and not isinstance(mistral_url, six.string_types): raise RuntimeError('Mistral url should be string') @@ -58,6 +61,9 @@ class Client(object): if not mistral_url: mistral_url = "http://localhost:8989/v2" + if profile: + osprofiler.profiler.init(profile) + self.http_client = httpclient.HTTPClient( mistral_url, auth_token, diff --git a/mistralclient/shell.py b/mistralclient/shell.py index a86c0ec7..fa555ad7 100644 --- a/mistralclient/shell.py +++ b/mistralclient/shell.py @@ -142,18 +142,21 @@ class MistralShell(app.App): :paramtype extra_kwargs: dict """ argparse_kwargs = argparse_kwargs or {} + parser = argparse.ArgumentParser( description=description, add_help=False, formatter_class=OpenStackHelpFormatter, **argparse_kwargs ) + parser.add_argument( '--version', action='version', version='%(prog)s {0}'.format(version), help='Show program\'s version number and exit.' ) + parser.add_argument( '-v', '--verbose', action='count', @@ -161,12 +164,14 @@ class MistralShell(app.App): default=self.DEFAULT_VERBOSE_LEVEL, help='Increase verbosity of output. Can be repeated.', ) + parser.add_argument( '--log-file', action='store', default=None, help='Specify a file to log output. Disabled by default.', ) + parser.add_argument( '-q', '--quiet', action='store_const', @@ -174,6 +179,7 @@ class MistralShell(app.App): const=0, help='Suppress output except warnings and errors.', ) + parser.add_argument( '-h', '--help', action=HelpAction, @@ -181,12 +187,14 @@ class MistralShell(app.App): default=self, # tricky help="Show this help message and exit.", ) + parser.add_argument( '--debug', default=False, action='store_true', help='Show tracebacks on errors.', ) + parser.add_argument( '--os-mistral-url', action='store', @@ -194,6 +202,7 @@ class MistralShell(app.App): default=c.env('OS_MISTRAL_URL'), help='Mistral API host (Env: OS_MISTRAL_URL)' ) + parser.add_argument( '--os-mistral-version', action='store', @@ -202,6 +211,7 @@ class MistralShell(app.App): help='Mistral API version (default = v2) (Env: ' 'OS_MISTRAL_VERSION)' ) + parser.add_argument( '--os-mistral-service-type', action='store', @@ -211,6 +221,7 @@ class MistralShell(app.App): 'keystone-endpoint) (default = workflowv2) (Env: ' 'OS_MISTRAL_SERVICE_TYPE)' ) + parser.add_argument( '--os-mistral-endpoint-type', action='store', @@ -220,6 +231,7 @@ class MistralShell(app.App): 'keystone-endpoint) (default = publicURL) (Env: ' 'OS_MISTRAL_ENDPOINT_TYPE)' ) + parser.add_argument( '--os-username', action='store', @@ -227,6 +239,7 @@ class MistralShell(app.App): default=c.env('OS_USERNAME', default='admin'), help='Authentication username (Env: OS_USERNAME)' ) + parser.add_argument( '--os-password', action='store', @@ -234,6 +247,7 @@ class MistralShell(app.App): default=c.env('OS_PASSWORD'), help='Authentication password (Env: OS_PASSWORD)' ) + parser.add_argument( '--os-tenant-id', action='store', @@ -241,6 +255,7 @@ class MistralShell(app.App): default=c.env('OS_TENANT_ID'), help='Authentication tenant identifier (Env: OS_TENANT_ID)' ) + parser.add_argument( '--os-tenant-name', action='store', @@ -248,6 +263,7 @@ class MistralShell(app.App): default=c.env('OS_TENANT_NAME', 'Default'), help='Authentication tenant name (Env: OS_TENANT_NAME)' ) + parser.add_argument( '--os-auth-token', action='store', @@ -255,6 +271,7 @@ class MistralShell(app.App): default=c.env('OS_AUTH_TOKEN'), help='Authentication token (Env: OS_AUTH_TOKEN)' ) + parser.add_argument( '--os-auth-url', action='store', @@ -262,6 +279,7 @@ class MistralShell(app.App): default=c.env('OS_AUTH_URL'), help='Authentication URL (Env: OS_AUTH_URL)' ) + parser.add_argument( '--os-cacert', action='store', @@ -269,6 +287,7 @@ class MistralShell(app.App): default=c.env('OS_CACERT'), help='Authentication CA Certificate (Env: OS_CACERT)' ) + parser.add_argument( '--insecure', action='store_true', @@ -277,6 +296,20 @@ class MistralShell(app.App): help='Disables SSL/TLS certificate verification ' '(Env: MISTRALCLIENT_INSECURE)' ) + + parser.add_argument( + '--profile', + dest='profile', + metavar='HMAC_KEY', + help='HMAC key to use for encrypting context data for performance ' + 'profiling of operation. This key should be one of the ' + 'values configured for the osprofiler middleware in mistral, ' + 'it is specified in the profiler section of the mistral ' + 'configuration (i.e. /etc/mistral/mistral.conf). Without the ' + 'key, profiling will not be triggered even if osprofiler is ' + 'enabled on the server side.' + ) + return parser def initialize_app(self, argv): @@ -310,7 +343,8 @@ class MistralShell(app.App): service_type=self.options.service_type, auth_token=self.options.token, cacert=self.options.cacert, - insecure=self.options.insecure + insecure=self.options.insecure, + profile=self.options.profile ) # Adding client_manager variable to make mistral client work with diff --git a/mistralclient/tests/unit/test_client.py b/mistralclient/tests/unit/test_client.py index c3a90434..e0809d34 100644 --- a/mistralclient/tests/unit/test_client.py +++ b/mistralclient/tests/unit/test_client.py @@ -20,12 +20,15 @@ import uuid import mock import testtools +import osprofiler.profiler + from mistralclient.api import client AUTH_HTTP_URL = 'http://localhost:35357/v3' AUTH_HTTPS_URL = AUTH_HTTP_URL.replace('http', 'https') MISTRAL_HTTP_URL = 'http://localhost:8989/v2' MISTRAL_HTTPS_URL = MISTRAL_HTTP_URL.replace('http', 'https') +PROFILER_HMAC_KEY = 'SECRET_HMAC_KEY' class BaseClientTests(testtools.TestCase): @@ -175,3 +178,38 @@ class BaseClientTests(testtools.TestCase): os.unlink(path) self.assertTrue(log_warning_mock.called) + + @mock.patch('keystoneclient.v3.client.Client') + @mock.patch('mistralclient.api.httpclient.HTTPClient') + def test_mistral_profile_enabled(self, mock, keystone_client_mock): + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + expected_args = ( + MISTRAL_HTTP_URL, + keystone_client_instance.auth_token, + keystone_client_instance.project_id, + keystone_client_instance.user_id + ) + + expected_kwargs = { + 'cacert': None, + 'insecure': False + } + + client.client( + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL, + profile=PROFILER_HMAC_KEY + ) + + self.assertTrue(mock.called) + self.assertEqual(mock.call_args[0], expected_args) + self.assertDictEqual(mock.call_args[1], expected_kwargs) + + profiler = osprofiler.profiler.get() + + self.assertEqual(profiler.hmac_key, PROFILER_HMAC_KEY) diff --git a/mistralclient/tests/unit/test_httpclient.py b/mistralclient/tests/unit/test_httpclient.py index a3341e41..43623e46 100644 --- a/mistralclient/tests/unit/test_httpclient.py +++ b/mistralclient/tests/unit/test_httpclient.py @@ -19,6 +19,9 @@ import mock import requests import testtools +from osprofiler import _utils as osprofiler_utils +import osprofiler.profiler + from mistralclient.api import httpclient API_BASE_URL = 'http://localhost:8989/v2' @@ -29,6 +32,8 @@ EXPECTED_URL = API_BASE_URL + API_URL AUTH_TOKEN = str(uuid.uuid4()) PROJECT_ID = str(uuid.uuid4()) USER_ID = str(uuid.uuid4()) +PROFILER_HMAC_KEY = 'SECRET_HMAC_KEY' +PROFILER_TRACE_ID = str(uuid.uuid4()) EXPECTED_AUTH_HEADERS = { 'x-auth-token': AUTH_TOKEN, @@ -65,6 +70,7 @@ class HTTPClientTest(testtools.TestCase): def setUp(self): super(HTTPClientTest, self).setUp() + osprofiler.profiler.init(None) self.client = httpclient.HTTPClient( API_BASE_URL, AUTH_TOKEN, @@ -103,6 +109,42 @@ class HTTPClientTest(testtools.TestCase): **expected_options ) + @mock.patch.object( + osprofiler.profiler._Profiler, + 'get_base_id', + mock.MagicMock(return_value=PROFILER_TRACE_ID) + ) + @mock.patch.object( + osprofiler.profiler._Profiler, + 'get_id', + mock.MagicMock(return_value=PROFILER_TRACE_ID) + ) + @mock.patch.object( + requests, + 'get', + mock.MagicMock(return_value=FakeResponse('get', EXPECTED_URL, 200)) + ) + def test_get_request_options_with_profile_enabled(self): + osprofiler.profiler.init(PROFILER_HMAC_KEY) + + data = {'base_id': PROFILER_TRACE_ID, 'parent_id': PROFILER_TRACE_ID} + signed_data = osprofiler_utils.signed_pack(data, PROFILER_HMAC_KEY) + + headers = { + 'X-Trace-Info': signed_data[0], + 'X-Trace-HMAC': signed_data[1] + } + + self.client.get(API_URL) + + expected_options = copy.deepcopy(EXPECTED_REQ_OPTIONS) + expected_options['headers'].update(headers) + + requests.get.assert_called_with( + EXPECTED_URL, + **expected_options + ) + @mock.patch.object( requests, 'post', diff --git a/mistralclient/tests/unit/test_shell.py b/mistralclient/tests/unit/test_shell.py index 2e7e373f..e6b07197 100644 --- a/mistralclient/tests/unit/test_shell.py +++ b/mistralclient/tests/unit/test_shell.py @@ -109,3 +109,10 @@ class TestShell(base.BaseShellTests): self.assertTrue(mock.called) params = mock.call_args self.assertEqual('http://localhost:35357/v3', params[1]['auth_url']) + + @mock.patch('mistralclient.api.client.client') + def test_profile(self, mock): + self.shell('--profile=SECRET_HMAC_KEY workbook-list') + self.assertTrue(mock.called) + params = mock.call_args + self.assertEqual('SECRET_HMAC_KEY', params[1]['profile']) diff --git a/requirements.txt b/requirements.txt index 08f30139..4d4b2985 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. cliff!=1.16.0,!=1.17.0,>=1.15.0 # Apache-2.0 +osprofiler>=1.3.0 # Apache-2.0 pbr>=1.6 # Apache-2.0 python-keystoneclient!=1.8.0,!=2.1.0,>=1.7.0 # Apache-2.0 python-openstackclient>=2.1.0 # Apache-2.0