From 674abd2f4c0acbf281d3642fae0008b1fd98d181 Mon Sep 17 00:00:00 2001 From: nasc17 <69922333+nasc17@users.noreply.github.com> Date: Mon, 25 Sep 2023 08:24:09 -0700 Subject: [PATCH 01/11] add full error with more detail (#479) --- ossdbtoolsservice/driver/types/psycopg_driver.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ossdbtoolsservice/driver/types/psycopg_driver.py b/ossdbtoolsservice/driver/types/psycopg_driver.py index 92341efbc..8cd2711a9 100644 --- a/ossdbtoolsservice/driver/types/psycopg_driver.py +++ b/ossdbtoolsservice/driver/types/psycopg_driver.py @@ -297,14 +297,14 @@ def get_error_message(self, error) -> str: """ Get the message from DatabaseError instance """ - # If error.diag.message_primary is not None, return it. - if error.diag and error.diag.message_primary: - return error.diag.message_primary - # If error.args exists and has at least one element, return the first element as the error message. - elif hasattr(error, 'args') and error.args and len(error.args) > 0: + if hasattr(error, 'args') and error.args and len(error.args) > 0: return error.args[0] + # If error.diag.message_primary is not None, return it. + elif error.diag and error.diag.message_primary: + return error.diag.message_primary + # If neither is available, return a generic error message. else: return "An unspecified database error occurred." From d7e9b0e4f9e3e97515b158b531d3d18a9b4eef12 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 12:24:53 -0400 Subject: [PATCH 02/11] initial commit --- .../connection/connection_service.py | 32 ++++++--- ossdbtoolsservice/exception/__init__.py | 4 ++ ossdbtoolsservice/exception/constants.py | 67 +++++++++++++++++++ ossdbtoolsservice/utils/__init__.py | 4 +- ossdbtoolsservice/utils/telemetry.py | 38 +++++++++++ 5 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 ossdbtoolsservice/exception/__init__.py create mode 100644 ossdbtoolsservice/exception/constants.py create mode 100644 ossdbtoolsservice/utils/telemetry.py diff --git a/ossdbtoolsservice/connection/connection_service.py b/ossdbtoolsservice/connection/connection_service.py index a72adfb75..29704d9ff 100644 --- a/ossdbtoolsservice/connection/connection_service.py +++ b/ossdbtoolsservice/connection/connection_service.py @@ -24,11 +24,14 @@ ) from ossdbtoolsservice.hosting import RequestContext, ServiceProvider -from ossdbtoolsservice.utils import constants +from ossdbtoolsservice.utils import constants as utils_constants +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants from ossdbtoolsservice.utils.cancellation import CancellationToken from ossdbtoolsservice.driver import ServerConnection, ConnectionManager + class ConnectionInfo(object): """Information pertaining to a unique connection instance""" @@ -90,7 +93,7 @@ def register(self, service_provider: ServiceProvider): self._service_provider.server.set_request_handler(GET_CONNECTION_STRING_REQUEST, self.handle_get_connection_string_request) # PUBLIC METHODS ####################################################### - def connect(self, params: ConnectRequestParams) -> Optional[ConnectionCompleteParams]: + def connect(self, params: ConnectRequestParams, request_context: RequestContext = None) -> Optional[ConnectionCompleteParams]: """ Open a connection using the given connection information. @@ -121,12 +124,12 @@ def connect(self, params: ConnectRequestParams) -> Optional[ConnectionCompletePa # Get the type of server and config provider_name = self._service_provider.provider - config = self._service_provider[constants.WORKSPACE_SERVICE_NAME].configuration + config = self._service_provider[utils_constants.WORKSPACE_SERVICE_NAME].configuration try: # Get connection to DB server using the provided connection params connection: ServerConnection = ConnectionManager(provider_name, config, params.connection.options).get_connection() except Exception as err: - return _build_connection_response_error(connection_info, params.type, err) + return _build_connection_response_error(connection_info, params.type, request_context, err) finally: # Remove this thread's cancellation token if needed with self._cancellation_lock: @@ -141,7 +144,7 @@ def connect(self, params: ConnectRequestParams) -> Optional[ConnectionCompletePa # The connection was not canceled, so add the connection and respond connection_info.add_connection(params.type, connection) - self._notify_on_connect(params.type, connection_info) + self._notify_on_connect(params.type, connection_info, request_context) return _build_connection_response(connection_info, params.type) def disconnect(self, owner_uri: str, connection_type: Optional[ConnectionType]) -> bool: @@ -157,7 +160,7 @@ def disconnect(self, owner_uri: str, connection_type: Optional[ConnectionType]) connection_info = self.owner_to_connection_map.get(owner_uri) return self._close_connections(connection_info, connection_type) if connection_info is not None else False - def get_connection(self, owner_uri: str, connection_type: ConnectionType) -> Optional[ServerConnection]: + def get_connection(self, owner_uri: str, connection_type: ConnectionType, request_context: RequestContext = None) -> Optional[ServerConnection]: """ Get a connection for the given owner URI and connection type @@ -168,7 +171,7 @@ def get_connection(self, owner_uri: str, connection_type: ConnectionType) -> Opt raise ValueError('No connection associated with given owner URI') if not connection_info.has_connection(connection_type) or not connection_info.get_connection(connection_type).open: - self.connect(ConnectRequestParams(connection_info.details, owner_uri, connection_type)) + self.connect(ConnectRequestParams(connection_info.details, owner_uri, connection_type), request_context) return connection_info.get_connection(connection_type) def register_on_connect_callback(self, task: Callable[[ConnectionInfo], None]) -> None: @@ -199,8 +202,13 @@ def handle_list_databases(self, request_context: RequestContext, params: ListDat """List all databases on the server that the given URI has a connection to""" connection = None try: - connection = self.get_connection(params.owner_uri, ConnectionType.DEFAULT) + connection = self.get_connection(params.owner_uri, ConnectionType.DEFAULT, request_context) except ValueError as err: + send_error_telemetry_notification( + request_context, + error_constants.CONNECTION, + error_constants.LIST_DATABASES_CONNECTION_VALUE_ERROR, + error_constants.LIST_DATABASE_GET_CONNECTION_VALUE_ERROR) request_context.send_error(str(err)) return @@ -261,14 +269,14 @@ def _connect_and_respond(self, request_context: RequestContext, params: ConnectR if response is not None: request_context.send_notification(CONNECTION_COMPLETE_METHOD, response) - def _notify_on_connect(self, conn_type: ConnectionType, info: ConnectionInfo) -> None: + def _notify_on_connect(self, conn_type: ConnectionType, info: ConnectionInfo, request_context: RequestContext = None) -> None: """ Sends a notification to any listeners that a new connection has been established. Only sent if the connection is a new, defalt connection """ if (conn_type == ConnectionType.DEFAULT): for callback in self._on_connect_callbacks: - callback(info) + callback(info, request_context) @staticmethod def _close_connections(connection_info: ConnectionInfo, connection_type=None): @@ -318,7 +326,7 @@ def _build_connection_response(connection_info: ConnectionInfo, connection_type: return response -def _build_connection_response_error(connection_info: ConnectionInfo, connection_type: ConnectionType, err)\ +def _build_connection_response_error(connection_info: ConnectionInfo, connection_type: ConnectionType, request_context: RequestContext, err: Exception)\ -> ConnectionCompleteParams: """Build a connection complete response object""" response: ConnectionCompleteParams = ConnectionCompleteParams() @@ -342,6 +350,8 @@ def _build_connection_response_error(connection_info: ConnectionInfo, connection response.messages = errorMessage response.error_message = errorMessage + send_error_telemetry_notification(request_context, error_constants.CONNECTION, error_constants.BUILD_CONNECTION_ERROR, error_constants.BUILD_CONNECTION_ERROR_CODE) + return response diff --git a/ossdbtoolsservice/exception/__init__.py b/ossdbtoolsservice/exception/__init__.py new file mode 100644 index 000000000..667942297 --- /dev/null +++ b/ossdbtoolsservice/exception/__init__.py @@ -0,0 +1,4 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- \ No newline at end of file diff --git a/ossdbtoolsservice/exception/constants.py b/ossdbtoolsservice/exception/constants.py new file mode 100644 index 000000000..8a13031cb --- /dev/null +++ b/ossdbtoolsservice/exception/constants.py @@ -0,0 +1,67 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +# This module holds the error resource constants for ossdb tools service errors + +# INTERNAL ERROR CODES. According to the standard, the first two characters of an error code denote a class of errors, while the last three characters indicate a specific condition within that class +# Read more here: https://www.postgresql.org/docs/current/errcodes-appendix.html +REQUEST_METHOD_PROCESSING_UNHANDLED_EXCEPTION = 'AD004' +EXECUTE_QUERY_GET_CONNECTION_ERROR = 'AD005' +EXECUTE_DEPLOY_GET_CONNECTION_ERROR = 'AD006' +CANCEL_QUERY_ERROR = 'AD007' +DISPOSE_QUERY_REQUEST_ERROR = 'AD008' +UNSUPPORTED_REQUEST_METHOD = 'AD009' +LIST_DATABASE_GET_CONNECTION_VALUE_ERROR = 'AD010' +LIST_DATABASE_ERROR = 'AD011' +EDIT_DATA_CUSTOM_QUERY_UNSUPPORTED_ERROR = 'AD012' +EDIT_DATA_COMMIT_FAILURE = 'AD013' +EDIT_DATA_SESSION_NOT_FOUND = 'AD014' +EDIT_DATA_SESSION_OPERATION_FAILURE = 'AD015' +GET_METADATA_FAILURE = 'AD016' +OBJECT_EXPLORER_CREATE_SESSION_ERROR = 'AD017' +OBJECT_EXPLORER_CLOSE_SESSION_ERROR = 'AD018' +OBJECT_EXPLORER_EXPAND_NODE_ERROR = 'AD019' +ANOTHER_QUERY_EXECUTING_ERROR = 'AD020' +DISPOSE_REQUEST_NO_QUERY_ERROR = 'AD021' +SAVE_QUERY_RESULT_ERROR = 'AD022' +SCRIPTAS_REQUEST_ERROR = 'AD023' +UNKNOWN_CONNECTION_ERROR = 'AD024' +PSYCOPG_DRIVER_UNKNOWN_ERROR_CODE = 'AD025' +NEW_DATABASE_GET_CHARSETS_ERROR_CODE = 'AD026' +NEW_DATABASE_GET_COLLATIONS_ERROR_CODE = 'AD027' +NEW_DATABASE_CREATE_ERROR_CODE = 'AD028' +BUILD_CONNECTION_ERROR_CODE = 'AD029' + +# ErrorTelemetryViews +CONNECTION = 'Connection' +EDIT_DATA = 'Edit Data' +JSON_RPC = 'Json Rpc' +METADATA = 'Metadata' +OBJECT_EXPLORER = 'Object Explorer' +QUERY_EXECUTION = 'Query Execution' +SCRIPTING = 'Scripting' + +# ErrorTelmetryNames +LIST_DATABASES_CONNECTION_VALUE_ERROR = 'List Databases Connection Value Error' +LIST_DATABASES_ERROR = 'List Databases Error' +BUILD_CONNECTION_ERROR = 'Build Connection Error' +EDIT_DATA_CUSTOM_QUERY = 'Edit Data Custom Query Unsupported' +EDIT_DATA_COMMIT = 'Edit Data Commit Failure' +EDIT_DATA_SESSION_NOT_FOUND = 'Edit Data Session Not Found' +EDIT_DATA_SESSION_OPERATION = 'Edit Data Session Operation Failure' +UNSUPPORTED_REQUEST = 'Unsupported Request Method' +REQUEST_METHOD_PROCESSING = 'Request Method Processing Unhandled Exception' +GET_METADATA_FAILURE = 'Get Metadata Failure' +OBJECT_EXPLORER_CREATE_SESSION = 'Object Explorer Create Session Error' +OBJECT_EXPLORER_CLOSE_SESSION = 'Object Explorer Close Session Error' +OBJECT_EXPLORER_EXPAND_NODE = 'Object Explorer Expand Node Error' +EXECUTE_QUERY_GET_CONNECTION = 'Execute Query Get Connection Error' +EXECUTE_DEPLOY_GET_CONNECTION = 'Execute Deploy Get Connection Error' +ANOTHER_QUERY_EXECUTING = 'Another Query Executing Error' +CANCEL_QUERY = 'Cancel Query' +DISPOSE_QUERY_NO_QUERY = 'Dispose Query No Query Error' +DISPOSE_QUERY_REQUEST = 'Dispose Query Request Error' +SAVE_QUERY_RESULT = 'Save Query Result Error' +SCRIPT_AS_REQUEST = 'Script As Request Error' diff --git a/ossdbtoolsservice/utils/__init__.py b/ossdbtoolsservice/utils/__init__.py index 6f1a9898b..c7c0eab76 100644 --- a/ossdbtoolsservice/utils/__init__.py +++ b/ossdbtoolsservice/utils/__init__.py @@ -12,6 +12,7 @@ import ossdbtoolsservice.utils.thread import ossdbtoolsservice.utils.time import ossdbtoolsservice.utils.validate # noqa +import ossdbtoolsservice.utils.telemetry __all__ = [ 'cancellation', @@ -20,5 +21,6 @@ 'serialization', 'thread', 'time', - 'validate' + 'validate', + 'telemetry' ] diff --git a/ossdbtoolsservice/utils/telemetry.py b/ossdbtoolsservice/utils/telemetry.py new file mode 100644 index 000000000..1b8bedfb7 --- /dev/null +++ b/ossdbtoolsservice/utils/telemetry.py @@ -0,0 +1,38 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +from ossdbtoolsservice.hosting.json_rpc_server import RequestContext +from ossdbtoolsservice.serialization import Serializable + +class TelemetryParams(Serializable): + """Parameters to be sent back with a telemetry event""" + + def __init__(self, eventName: str, properties: dict(), measures: dict() = None): + self.params = { + 'eventName': eventName, + 'properties': properties, + 'measures': measures + } + + +def send_error_telemetry_notification(request_context: RequestContext, view: str, name: str, errorCode: int): + if request_context is not None: + request_context.send_notification( + method = TELEMETRY_NOTIFICATION, + params = TelemetryParams( + TELEMETRY_ERROR_EVENT, + { + 'view': view, + 'name': name, + 'errorCode': str(errorCode) + } + ) + ) + +# Method name listened by client +TELEMETRY_NOTIFICATION = "telemetry/pgevent" + +# Telemetry Event Name for Errors +TELEMETRY_ERROR_EVENT = "telemetry/error" From 85a019050f0fb024117d9fcde60bcdfa85ee39a6 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 12:31:34 -0400 Subject: [PATCH 03/11] language service --- ossdbtoolsservice/language/language_service.py | 11 ++++++----- ossdbtoolsservice/language/operations_queue.py | 12 ++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ossdbtoolsservice/language/language_service.py b/ossdbtoolsservice/language/language_service.py index a8b965734..2b037e68a 100644 --- a/ossdbtoolsservice/language/language_service.py +++ b/ossdbtoolsservice/language/language_service.py @@ -242,9 +242,9 @@ def do_send_default_empty_response(): do_send_default_empty_response() # SERVICE NOTIFICATION HANDLERS ##################################################### - def on_connect(self, conn_info: ConnectionInfo) -> threading.Thread: + def on_connect(self, conn_info: ConnectionInfo, request_context: RequestContext) -> threading.Thread: """Set up intellisense cache on connection to a new database""" - return utils.thread.run_as_thread(self._build_intellisense_cache_thread, conn_info) + return utils.thread.run_as_thread(self._build_intellisense_cache_thread, conn_info, request_context) # PROPERTIES ########################################################### @property @@ -278,12 +278,12 @@ def is_valid_uri(self, uri: str) -> bool: """ return uri in self._valid_uri - def _build_intellisense_cache_thread(self, conn_info: ConnectionInfo) -> None: + def _build_intellisense_cache_thread(self, conn_info: ConnectionInfo, request_context: RequestContext) -> None: # TODO build the cache. For now, sending intellisense ready as a test scriptparseinfo: ScriptParseInfo = self.get_script_parse_info(conn_info.owner_uri, create_if_not_exists=True) if scriptparseinfo is not None: # This is a connection for an actual script in the workspace. Build the intellisense cache for it - connection_context: ConnectionContext = self.operations_queue.add_connection_context(conn_info, False) + connection_context: ConnectionContext = self.operations_queue.add_connection_context(conn_info, False, request_context) # Wait until the intellisense is completed before sending back the message and caching the key connection_context.intellisense_complete.wait() scriptparseinfo.connection_key = connection_context.key @@ -366,7 +366,8 @@ def send_definition_using_connected_completions(self, request_context: RequestCo matching_completion = next(completion for completion in completions if completion.display == word_under_cursor) if matching_completion: connection = self._connection_service.get_connection(params.text_document.uri, - ConnectionType.QUERY) + ConnectionType.QUERY, + request_context) scripter_instance = scripter.Scripter(connection) object_metadata = ObjectMetadata(None, None, matching_completion.display_meta, matching_completion.display, diff --git a/ossdbtoolsservice/language/operations_queue.py b/ossdbtoolsservice/language/operations_queue.py index cc5a33102..0a994e176 100644 --- a/ossdbtoolsservice/language/operations_queue.py +++ b/ossdbtoolsservice/language/operations_queue.py @@ -13,7 +13,7 @@ import ossdbtoolsservice.utils as utils from ossdbtoolsservice.connection import ConnectionInfo, ConnectionService from ossdbtoolsservice.connection.contracts import ConnectRequestParams, ConnectionType -from ossdbtoolsservice.hosting import ServiceProvider +from ossdbtoolsservice.hosting import ServiceProvider, RequestContext from ossdbtoolsservice.language.completion_refresher import CompletionRefresher from ossdbtoolsservice.driver import ServerConnection @@ -117,7 +117,7 @@ def has_connection_context(self, conn_info: ConnectionInfo) -> bool: key: str = OperationsQueue.create_key(conn_info) return key in self._context_map - def add_connection_context(self, conn_info: ConnectionInfo, overwrite=False) -> ConnectionContext: + def add_connection_context(self, conn_info: ConnectionInfo, request_context: RequestContext, overwrite=False) -> ConnectionContext: """ Adds a connection context and returns the notification event. If a connection queue exists alread, will overwrite if necesary @@ -135,7 +135,7 @@ def add_connection_context(self, conn_info: ConnectionInfo, overwrite=False) -> return context # Create the context and start refresh context = ConnectionContext(key, logger) - conn = self._create_connection(key, conn_info) + conn = self._create_connection(key, conn_info, request_context) context.refresh_metadata(conn) self._context_map[key] = context return context @@ -162,15 +162,15 @@ def create_key(cls, conn_info: ConnectionInfo) -> str: """ return '{0}|{1}|{2}'.format(conn_info.details.server_name, conn_info.details.database_name, conn_info.details.user_name) - def _create_connection(self, connection_key: str, conn_info: ConnectionInfo) -> Optional[ServerConnection]: + def _create_connection(self, connection_key: str, conn_info: ConnectionInfo, request_context: RequestContext) -> Optional[ServerConnection]: conn_service = self._connection_service key_uri = INTELLISENSE_URI + connection_key connect_request = ConnectRequestParams(conn_info.details, key_uri, ConnectionType.INTELLISENSE) - connect_result = conn_service.connect(connect_request) + connect_result = conn_service.connect(connect_request, request_context) if connect_result.error_message is not None: raise RuntimeError(connect_result.error_message) - connection = conn_service.get_connection(key_uri, ConnectionType.INTELLISENSE) + connection = conn_service.get_connection(key_uri, ConnectionType.INTELLISENSE, request_context) return connection def _process_operations(self): From ca21a9fc9f66dc861e164a930ff885cbc8c5a0f3 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 13:01:11 -0400 Subject: [PATCH 04/11] add telemetry error for several services --- ossdbtoolsservice/admin/admin_service.py | 3 +- ossdbtoolsservice/exception/constants.py | 3 +- ossdbtoolsservice/hosting/json_rpc_server.py | 12 +++++ .../metadata/metadata_service.py | 13 +++-- .../object_explorer_service.py | 32 ++++++++++--- .../query_execution_service.py | 48 +++++++++++++++---- ossdbtoolsservice/utils/telemetry.py | 2 +- 7 files changed, 93 insertions(+), 20 deletions(-) diff --git a/ossdbtoolsservice/admin/admin_service.py b/ossdbtoolsservice/admin/admin_service.py index 803b8f49a..ad36b883e 100644 --- a/ossdbtoolsservice/admin/admin_service.py +++ b/ossdbtoolsservice/admin/admin_service.py @@ -9,6 +9,7 @@ from ossdbtoolsservice.hosting import RequestContext, ServiceProvider from ossdbtoolsservice.utils import constants from ossdbtoolsservice.driver import ServerConnection +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification class AdminService(object): @@ -33,7 +34,7 @@ def register(self, service_provider: ServiceProvider): def _handle_get_database_info_request(self, request_context: RequestContext, params: GetDatabaseInfoParameters) -> None: # Retrieve the connection from the connection service connection_service = self._service_provider[constants.CONNECTION_SERVICE_NAME] - connection: ServerConnection = connection_service.get_connection(params.owner_uri, ConnectionType.DEFAULT) + connection: ServerConnection = connection_service.get_connection(params.owner_uri, ConnectionType.DEFAULT, request_context) # Get database owner owner_result = connection.get_database_owner() diff --git a/ossdbtoolsservice/exception/constants.py b/ossdbtoolsservice/exception/constants.py index 8a13031cb..959d7f57e 100644 --- a/ossdbtoolsservice/exception/constants.py +++ b/ossdbtoolsservice/exception/constants.py @@ -33,6 +33,7 @@ NEW_DATABASE_GET_COLLATIONS_ERROR_CODE = 'AD027' NEW_DATABASE_CREATE_ERROR_CODE = 'AD028' BUILD_CONNECTION_ERROR_CODE = 'AD029' +LIST_METADATA_FAILURE_ERROR_CODE = 'AD030' # ErrorTelemetryViews CONNECTION = 'Connection' @@ -53,7 +54,7 @@ EDIT_DATA_SESSION_OPERATION = 'Edit Data Session Operation Failure' UNSUPPORTED_REQUEST = 'Unsupported Request Method' REQUEST_METHOD_PROCESSING = 'Request Method Processing Unhandled Exception' -GET_METADATA_FAILURE = 'Get Metadata Failure' +LIST_METADATA_FAILURE = 'List Metadata Failure' OBJECT_EXPLORER_CREATE_SESSION = 'Object Explorer Create Session Error' OBJECT_EXPLORER_CLOSE_SESSION = 'Object Explorer Close Session Error' OBJECT_EXPLORER_EXPAND_NODE = 'Object Explorer Expand Node Error' diff --git a/ossdbtoolsservice/hosting/json_rpc_server.py b/ossdbtoolsservice/hosting/json_rpc_server.py index 6a3dc6ee2..0f4e0a0fd 100644 --- a/ossdbtoolsservice/hosting/json_rpc_server.py +++ b/ossdbtoolsservice/hosting/json_rpc_server.py @@ -10,6 +10,8 @@ from ossdbtoolsservice.hosting.json_message import JSONRPCMessage, JSONRPCMessageType from ossdbtoolsservice.hosting.json_reader import JSONRPCReader from ossdbtoolsservice.hosting.json_writer import JSONRPCWriter +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants class JSONRPCServer: @@ -267,6 +269,11 @@ def _dispatch_message(self, message): # Make sure we got a handler for the request if handler is None: # TODO: Localize? + send_error_telemetry_notification( + request_context, + error_constants.JSON_RPC, + error_constants.UNSUPPORTED_REQUEST, + error_constants.UNSUPPORTED_REQUEST_METHOD) request_context.send_error(f'Requested method is unsupported: {message.message_method}') if self._logger is not None: self._logger.warn('Requested method is unsupported: %s', message.message_method) @@ -285,6 +292,11 @@ def _dispatch_message(self, message): error_message = f'Unhandled exception while handling request method {message.message_method}: "{e}"' # TODO: Localize if self._logger is not None: self._logger.exception(error_message) + send_error_telemetry_notification( + request_context, + error_constants.JSON_RPC, + error_constants.REQUEST_METHOD_PROCESSING, + error_constants.REQUEST_METHOD_PROCESSING_UNHANDLED_EXCEPTION) request_context.send_error(error_message, code=-32603) elif message.message_type is JSONRPCMessageType.Notification: if self._logger is not None: diff --git a/ossdbtoolsservice/metadata/metadata_service.py b/ossdbtoolsservice/metadata/metadata_service.py index df6f657c8..cd01b770e 100644 --- a/ossdbtoolsservice/metadata/metadata_service.py +++ b/ossdbtoolsservice/metadata/metadata_service.py @@ -12,6 +12,8 @@ from ossdbtoolsservice.metadata.contracts import ( MetadataListParameters, MetadataListResponse, METADATA_LIST_REQUEST, MetadataType, ObjectMetadata) from ossdbtoolsservice.utils import constants +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants # This query collects all the tables, views, and functions in all the schemas in the database(s)? PG_METADATA_QUERY = """ @@ -62,17 +64,22 @@ def _handle_metadata_list_request(self, request_context: RequestContext, params: def _metadata_list_worker(self, request_context: RequestContext, params: MetadataListParameters) -> None: try: - metadata = self._list_metadata(params.owner_uri) + metadata = self._list_metadata(params.owner_uri, request_context) request_context.send_response(MetadataListResponse(metadata)) except Exception as e: if self._service_provider.logger is not None: self._service_provider.logger.exception('Unhandled exception while executing the metadata list worker thread') + send_error_telemetry_notification( + error_constants.METADATA, + error_constants.LIST_METADATA_FAILURE, + error_constants.LIST_METADATA_FAILURE_ERROR_CODE + ) request_context.send_error('Unhandled exception while listing metadata: ' + str(e)) # TODO: Localize - def _list_metadata(self, owner_uri: str) -> List[ObjectMetadata]: + def _list_metadata(self, owner_uri: str, request_context: RequestContext) -> List[ObjectMetadata]: # Get current connection connection_service = self._service_provider[constants.CONNECTION_SERVICE_NAME] - connection: ServerConnection = connection_service.get_connection(owner_uri, ConnectionType.DEFAULT) + connection: ServerConnection = connection_service.get_connection(owner_uri, ConnectionType.DEFAULT, request_context) # Get the current database database_name = connection.database_name diff --git a/ossdbtoolsservice/object_explorer/object_explorer_service.py b/ossdbtoolsservice/object_explorer/object_explorer_service.py index 73a14b92b..2206e5d09 100644 --- a/ossdbtoolsservice/object_explorer/object_explorer_service.py +++ b/ossdbtoolsservice/object_explorer/object_explorer_service.py @@ -22,6 +22,8 @@ from ossdbtoolsservice.object_explorer.session import ObjectExplorerSession from ossdbtoolsservice.metadata.contracts import ObjectMetadata import ossdbtoolsservice.utils as utils +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants from pgsmo import Server as PGServer @@ -111,6 +113,12 @@ def _handle_create_session_request(self, request_context: RequestContext, params message = f'Failed to create OE session: {str(e)}' if self._service_provider.logger is not None: self._service_provider.logger.error(message) + send_error_telemetry_notification( + request_context, + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CREATE_SESSION, + error_constants.OBJECT_EXPLORER_CREATE_SESSION_ERROR + ) request_context.send_error(message) return @@ -148,6 +156,12 @@ def _handle_close_session_request(self, request_context: RequestContext, params: message = f'Failed to close OE session: {str(e)}' # TODO: Localize if self._service_provider.logger is not None: self._service_provider.logger.error(message) + send_error_telemetry_notification( + request_context, + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION_ERROR + ) request_context.send_error(message) def _handle_refresh_request(self, request_context: RequestContext, params: ExpandParameters) -> None: @@ -261,10 +275,16 @@ def _get_session(self, request_context: RequestContext, params: ExpandParameters message = f'Failed to expand node base: {str(e)}' # TODO: Localize if self._service_provider.logger is not None: self._service_provider.logger.error(message) + send_error_telemetry_notification( + request_context, + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_EXPAND_NODE, + error_constants.OBJECT_EXPLORER_EXPAND_NODE_ERROR + ) request_context.send_error(message) return - def _create_connection(self, session: ObjectExplorerSession, database_name: str) -> Optional[ServerConnection]: + def _create_connection(self, session: ObjectExplorerSession, database_name: str, request_context: RequestContext) -> Optional[ServerConnection]: conn_service = self._service_provider[utils.constants.CONNECTION_SERVICE_NAME] options = session.connection_details.options.copy() @@ -273,11 +293,11 @@ def _create_connection(self, session: ObjectExplorerSession, database_name: str) key_uri = session.id + database_name connect_request = ConnectRequestParams(conn_details, key_uri, ConnectionType.OBJECT_EXLPORER) - connect_result = conn_service.connect(connect_request) + connect_result = conn_service.connect(connect_request, request_context) if connect_result.error_message is not None: raise RuntimeError(connect_result.error_message) - connection = conn_service.get_connection(key_uri, ConnectionType.OBJECT_EXLPORER) + connection = conn_service.get_connection(key_uri, ConnectionType.OBJECT_EXLPORER, request_context) return connection def _initialize_session(self, request_context: RequestContext, session: ObjectExplorerSession): @@ -292,17 +312,17 @@ def _initialize_session(self, request_context: RequestContext, session: ObjectEx session.id, ConnectionType.OBJECT_EXLPORER ) - connect_result = conn_service.connect(connect_request) + connect_result = conn_service.connect(connect_request, request_context) if connect_result is None: raise RuntimeError('Connection was cancelled during connect') # TODO Localize if connect_result.error_message is not None: raise RuntimeError(connect_result.error_message) # Step 2: Get the connection to use for object explorer - connection = conn_service.get_connection(session.id, ConnectionType.OBJECT_EXLPORER) + connection = conn_service.get_connection(session.id, ConnectionType.OBJECT_EXLPORER, request_context) # Step 3: Create the Server object for the session and create the root node for the server - session.server = self._server(connection, functools.partial(self._create_connection, session)) + session.server = self._server(connection, functools.partial(self._create_connection, session, request_context=request_context)) metadata = ObjectMetadata(session.server.urn_base, None, 'Database', session.server.maintenance_db_name) node = NodeInfo() node.label = session.connection_details.database_name diff --git a/ossdbtoolsservice/query_execution/query_execution_service.py b/ossdbtoolsservice/query_execution/query_execution_service.py index 019c586f5..3202e07e7 100644 --- a/ossdbtoolsservice/query_execution/query_execution_service.py +++ b/ossdbtoolsservice/query_execution/query_execution_service.py @@ -37,6 +37,8 @@ from ossdbtoolsservice.connection.contracts import ConnectRequestParams from ossdbtoolsservice.connection.contracts import ConnectionType import ossdbtoolsservice.utils as utils +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants from ossdbtoolsservice.query.data_storage import ( FileStreamFactory, SaveAsCsvFileStreamFactory, SaveAsJsonFileStreamFactory, SaveAsExcelFileStreamFactory ) @@ -121,7 +123,7 @@ def _handle_simple_execute_request(self, request_context: RequestContext, params connection_service = self._service_provider[utils.constants.CONNECTION_SERVICE_NAME] connection_info = connection_service.get_connection_info(params.owner_uri) connection_service.connect(ConnectRequestParams(connection_info.details, new_owner_uri, ConnectionType.QUERY)) - new_connection = self._get_connection(new_owner_uri, ConnectionType.QUERY) + new_connection = self._get_connection(new_owner_uri, ConnectionType.QUERY, request_context) execute_params = ExecuteStringParams() execute_params.query = params.query_string @@ -175,11 +177,17 @@ def on_query_complete(query_complete_params): # Get a connection for the query try: - conn = self._get_connection(params.owner_uri, ConnectionType.QUERY) + conn = self._get_connection(params.owner_uri, ConnectionType.QUERY, request_context) except Exception as e: if self._service_provider.logger is not None: self._service_provider.logger.exception( 'Encountered exception while handling query request') # TODO: Localize + send_error_telemetry_notification( + request_context, + error_constants.QUERY_EXECUTION, + error_constants.EXECUTE_QUERY_GET_CONNECTION, + error_constants.EXECUTE_QUERY_GET_CONNECTION_ERROR + ) request_context.send_unhandled_error_response(e) return @@ -215,11 +223,17 @@ def on_query_complete(query_complete_params): # Get a connection for the query try: - conn = self._get_connection(params.owner_uri, ConnectionType.QUERY) + conn = self._get_connection(params.owner_uri, ConnectionType.QUERY, request_context) except Exception as e: if self._service_provider.logger is not None: self._service_provider.logger.exception( 'Encountered exception while handling query request') # TODO: Localize + send_error_telemetry_notification( + request_context, + error_constants.QUERY_EXECUTION, + error_constants.EXECUTE_DEPLOY_GET_CONNECTION, + error_constants.EXECUTE_DEPLOY_GET_CONNECTION_ERROR + ) request_context.send_unhandled_error_response(e) return @@ -312,17 +326,29 @@ def _handle_cancel_query_request(self, request_context: RequestContext, params: # Only need to do additional work to cancel the query # if it's currently running if query.execution_state is ExecutionState.EXECUTING: - self.cancel_query(params.owner_uri) + self.cancel_query(params.owner_uri, request_context) request_context.send_response(QueryCancelResult()) except Exception as e: if self._service_provider.logger is not None: self._service_provider.logger.exception(str(e)) + send_error_telemetry_notification( + request_context, + error_constants.QUERY_EXECUTION, + error_constants.CANCEL_QUERY, + error_constants.CANCEL_QUERY_ERROR + ) request_context.send_unhandled_error_response(e) def _handle_dispose_request(self, request_context: RequestContext, params: QueryDisposeParams): try: if params.owner_uri not in self.query_results: + send_error_telemetry_notification( + request_context, + error_constants.QUERY_EXECUTION, + error_constants.DISPOSE_QUERY_NO_QUERY, + error_constants.DISPOSE_REQUEST_NO_QUERY_ERROR + ) request_context.send_error(NO_QUERY_MESSAGE) # TODO: Localize return # Make sure to cancel the query first if it's not executed. @@ -333,11 +359,17 @@ def _handle_dispose_request(self, request_context: RequestContext, params: Query del self.query_results[params.owner_uri] request_context.send_response({}) except Exception as e: + send_error_telemetry_notification( + request_context, + error_constants.QUERY_EXECUTION, + error_constants.DISPOSE_QUERY_REQUEST, + error_constants.DISPOSE_QUERY_REQUEST_ERROR + ) request_context.send_unhandled_error_response(e) - def cancel_query(self, owner_uri: str): - conn = self._get_connection(owner_uri, ConnectionType.QUERY) - cancel_conn = self._get_connection(owner_uri, ConnectionType.QUERY_CANCEL) + def cancel_query(self, owner_uri: str, request_context: RequestContext): + conn = self._get_connection(owner_uri, ConnectionType.QUERY, request_context) + cancel_conn = self._get_connection(owner_uri, ConnectionType.QUERY_CANCEL, request_context) if conn is None or cancel_conn is None: raise LookupError('Could not find associated connection') # TODO: Localize @@ -373,7 +405,7 @@ def _execute_query_request_worker(self, worker_args: ExecuteRequestWorkerArgs, r query_complete_params = QueryCompleteNotificationParams(worker_args.owner_uri, batch_summaries) _check_and_fire(worker_args.on_query_complete, query_complete_params) - def _get_connection(self, owner_uri: str, connection_type: ConnectionType) -> ServerConnection: + def _get_connection(self, owner_uri: str, connection_type: ConnectionType, request_context: RequestContext) -> ServerConnection: """ Get a connection for the given owner URI and connection type from the connection service diff --git a/ossdbtoolsservice/utils/telemetry.py b/ossdbtoolsservice/utils/telemetry.py index 1b8bedfb7..3c17293c1 100644 --- a/ossdbtoolsservice/utils/telemetry.py +++ b/ossdbtoolsservice/utils/telemetry.py @@ -17,7 +17,7 @@ def __init__(self, eventName: str, properties: dict(), measures: dict() = None): } -def send_error_telemetry_notification(request_context: RequestContext, view: str, name: str, errorCode: int): +def send_error_telemetry_notification(request_context: RequestContext, view: str, name: str, errorCode): if request_context is not None: request_context.send_notification( method = TELEMETRY_NOTIFICATION, From 9361919285456f7c4333fb8e590eccdf56d338e8 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 13:06:54 -0400 Subject: [PATCH 05/11] scripting service telemetry --- ossdbtoolsservice/scripting/scripting_service.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ossdbtoolsservice/scripting/scripting_service.py b/ossdbtoolsservice/scripting/scripting_service.py index a42c6ec65..1d42a9300 100644 --- a/ossdbtoolsservice/scripting/scripting_service.py +++ b/ossdbtoolsservice/scripting/scripting_service.py @@ -13,6 +13,8 @@ ) from ossdbtoolsservice.connection.contracts import ConnectionType import ossdbtoolsservice.utils as utils +from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification +from ossdbtoolsservice.exception import constants as error_constants class ScriptingService(object): @@ -53,7 +55,7 @@ def _handle_script_as_request(self, request_context: RequestContext, params: Scr try: scripting_operation = params.operation connection_service = self._service_provider[utils.constants.CONNECTION_SERVICE_NAME] - connection = connection_service.get_connection(params.owner_uri, ConnectionType.QUERY) + connection = connection_service.get_connection(params.owner_uri, ConnectionType.QUERY, request_context) object_metadata = self.create_metadata(params) scripter = Scripter(connection) @@ -65,9 +67,15 @@ def _handle_script_as_request(self, request_context: RequestContext, params: Scr self._service_provider.logger.warn('Server closed the connection unexpectedly. Attempting to reconnect...') self._handle_script_as_request(request_context, params, True) else: - self._request_error(request_context, params, str(e)) + self._request_error(request_context, params, str(e), request_context) def _request_error(self, request_context: RequestContext, params: ScriptAsParameters, message: str): if self._service_provider.logger is not None: self._service_provider.logger.exception('Scripting operation failed') + send_error_telemetry_notification( + request_context, + error_constants.SCRIPTING, + error_constants.SCRIPT_AS_REQUEST, + error_constants.SCRIPTAS_REQUEST_ERROR + ) request_context.send_error(message, params) From 02271f790a1a29c737aa0ef124ae37a6bb09753c Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 13:36:47 -0400 Subject: [PATCH 06/11] pass flake --- ossdbtoolsservice/admin/admin_service.py | 1 - ossdbtoolsservice/connection/connection_service.py | 8 +++++--- ossdbtoolsservice/exception/__init__.py | 2 +- ossdbtoolsservice/exception/constants.py | 6 ++++-- ossdbtoolsservice/utils/__init__.py | 2 +- ossdbtoolsservice/utils/telemetry.py | 8 +++++--- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ossdbtoolsservice/admin/admin_service.py b/ossdbtoolsservice/admin/admin_service.py index ad36b883e..297228e11 100644 --- a/ossdbtoolsservice/admin/admin_service.py +++ b/ossdbtoolsservice/admin/admin_service.py @@ -9,7 +9,6 @@ from ossdbtoolsservice.hosting import RequestContext, ServiceProvider from ossdbtoolsservice.utils import constants from ossdbtoolsservice.driver import ServerConnection -from ossdbtoolsservice.utils.telemetry import send_error_telemetry_notification class AdminService(object): diff --git a/ossdbtoolsservice/connection/connection_service.py b/ossdbtoolsservice/connection/connection_service.py index 29704d9ff..fc4b1cb95 100644 --- a/ossdbtoolsservice/connection/connection_service.py +++ b/ossdbtoolsservice/connection/connection_service.py @@ -31,7 +31,6 @@ from ossdbtoolsservice.driver import ServerConnection, ConnectionManager - class ConnectionInfo(object): """Information pertaining to a unique connection instance""" @@ -350,8 +349,11 @@ def _build_connection_response_error(connection_info: ConnectionInfo, connection response.messages = errorMessage response.error_message = errorMessage - send_error_telemetry_notification(request_context, error_constants.CONNECTION, error_constants.BUILD_CONNECTION_ERROR, error_constants.BUILD_CONNECTION_ERROR_CODE) - + send_error_telemetry_notification( + request_context, error_constants.CONNECTION, + error_constants.BUILD_CONNECTION_ERROR, + error_constants.BUILD_CONNECTION_ERROR_CODE + ) return response diff --git a/ossdbtoolsservice/exception/__init__.py b/ossdbtoolsservice/exception/__init__.py index 667942297..34913fb39 100644 --- a/ossdbtoolsservice/exception/__init__.py +++ b/ossdbtoolsservice/exception/__init__.py @@ -1,4 +1,4 @@ # -------------------------------------------------------------------------------------------- # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- \ No newline at end of file +# -------------------------------------------------------------------------------------------- diff --git a/ossdbtoolsservice/exception/constants.py b/ossdbtoolsservice/exception/constants.py index 959d7f57e..76c27ee9b 100644 --- a/ossdbtoolsservice/exception/constants.py +++ b/ossdbtoolsservice/exception/constants.py @@ -4,9 +4,11 @@ # -------------------------------------------------------------------------------------------- # This module holds the error resource constants for ossdb tools service errors - -# INTERNAL ERROR CODES. According to the standard, the first two characters of an error code denote a class of errors, while the last three characters indicate a specific condition within that class +# According to the PostgreSQL standard, the first two characters of an error code denote a class of errors, while the +# last three characters indicate a specific condition within that class # Read more here: https://www.postgresql.org/docs/current/errcodes-appendix.html + +# INTERNAL ERROR CODES REQUEST_METHOD_PROCESSING_UNHANDLED_EXCEPTION = 'AD004' EXECUTE_QUERY_GET_CONNECTION_ERROR = 'AD005' EXECUTE_DEPLOY_GET_CONNECTION_ERROR = 'AD006' diff --git a/ossdbtoolsservice/utils/__init__.py b/ossdbtoolsservice/utils/__init__.py index c7c0eab76..28b053491 100644 --- a/ossdbtoolsservice/utils/__init__.py +++ b/ossdbtoolsservice/utils/__init__.py @@ -12,7 +12,7 @@ import ossdbtoolsservice.utils.thread import ossdbtoolsservice.utils.time import ossdbtoolsservice.utils.validate # noqa -import ossdbtoolsservice.utils.telemetry +import ossdbtoolsservice.utils.telemetry # noqa __all__ = [ 'cancellation', diff --git a/ossdbtoolsservice/utils/telemetry.py b/ossdbtoolsservice/utils/telemetry.py index 3c17293c1..371b8d732 100644 --- a/ossdbtoolsservice/utils/telemetry.py +++ b/ossdbtoolsservice/utils/telemetry.py @@ -6,6 +6,7 @@ from ossdbtoolsservice.hosting.json_rpc_server import RequestContext from ossdbtoolsservice.serialization import Serializable + class TelemetryParams(Serializable): """Parameters to be sent back with a telemetry event""" @@ -20,8 +21,8 @@ def __init__(self, eventName: str, properties: dict(), measures: dict() = None): def send_error_telemetry_notification(request_context: RequestContext, view: str, name: str, errorCode): if request_context is not None: request_context.send_notification( - method = TELEMETRY_NOTIFICATION, - params = TelemetryParams( + method=TELEMETRY_NOTIFICATION, + params=TelemetryParams( TELEMETRY_ERROR_EVENT, { 'view': view, @@ -29,7 +30,8 @@ def send_error_telemetry_notification(request_context: RequestContext, view: str 'errorCode': str(errorCode) } ) - ) + ) + # Method name listened by client TELEMETRY_NOTIFICATION = "telemetry/pgevent" From 54c03bd6bf4a95c9ccb7bc9bd00af89871c397b0 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 15:24:36 -0400 Subject: [PATCH 07/11] fixed some tests --- .../metadata/metadata_service.py | 1 + ossdbtoolsservice/utils/telemetry.py | 7 +++--- tests/language/test_language_service.py | 3 ++- tests/language/test_operations_queue.py | 17 +++++++++---- tests/metadata/test_metadata_service.py | 6 ++--- tests/scripting/test_scripting_service.py | 25 +++++++++++++++++-- 6 files changed, 44 insertions(+), 15 deletions(-) diff --git a/ossdbtoolsservice/metadata/metadata_service.py b/ossdbtoolsservice/metadata/metadata_service.py index cd01b770e..eaaac616c 100644 --- a/ossdbtoolsservice/metadata/metadata_service.py +++ b/ossdbtoolsservice/metadata/metadata_service.py @@ -70,6 +70,7 @@ def _metadata_list_worker(self, request_context: RequestContext, params: Metadat if self._service_provider.logger is not None: self._service_provider.logger.exception('Unhandled exception while executing the metadata list worker thread') send_error_telemetry_notification( + request_context, error_constants.METADATA, error_constants.LIST_METADATA_FAILURE, error_constants.LIST_METADATA_FAILURE_ERROR_CODE diff --git a/ossdbtoolsservice/utils/telemetry.py b/ossdbtoolsservice/utils/telemetry.py index 371b8d732..0e9d3a423 100644 --- a/ossdbtoolsservice/utils/telemetry.py +++ b/ossdbtoolsservice/utils/telemetry.py @@ -3,7 +3,6 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from ossdbtoolsservice.hosting.json_rpc_server import RequestContext from ossdbtoolsservice.serialization import Serializable @@ -18,7 +17,7 @@ def __init__(self, eventName: str, properties: dict(), measures: dict() = None): } -def send_error_telemetry_notification(request_context: RequestContext, view: str, name: str, errorCode): +def send_error_telemetry_notification(request_context, view: str, name: str, errorCode): if request_context is not None: request_context.send_notification( method=TELEMETRY_NOTIFICATION, @@ -34,7 +33,7 @@ def send_error_telemetry_notification(request_context: RequestContext, view: str # Method name listened by client -TELEMETRY_NOTIFICATION = "telemetry/pgevent" +TELEMETRY_NOTIFICATION = "telemetry/event" # Telemetry Event Name for Errors -TELEMETRY_ERROR_EVENT = "telemetry/error" +TELEMETRY_ERROR_EVENT = "telemetry/event/error" diff --git a/tests/language/test_language_service.py b/tests/language/test_language_service.py index 996673771..3bda844b9 100644 --- a/tests/language/test_language_service.py +++ b/tests/language/test_language_service.py @@ -235,11 +235,12 @@ def validate_success_notification(response: IntelliSenseReadyParams): refresher_mock = mock.MagicMock() refresh_method_mock = mock.MagicMock() + request_context: RequestContext = utils.MockRequestContext() refresher_mock.refresh = refresh_method_mock patch_path = 'ossdbtoolsservice.language.operations_queue.CompletionRefresher' with mock.patch(patch_path) as refresher_patch: refresher_patch.return_value = refresher_mock - task: threading.Thread = service.on_connect(conn_info) + task: threading.Thread = service.on_connect(conn_info, request_context) # And when refresh is "complete" refresh_method_mock.assert_called_once() callback = refresh_method_mock.call_args[0][0] diff --git a/tests/language/test_operations_queue.py b/tests/language/test_operations_queue.py index 1c154cf13..00f24fb3f 100644 --- a/tests/language/test_operations_queue.py +++ b/tests/language/test_operations_queue.py @@ -10,6 +10,7 @@ from unittest import mock from ossdbtoolsservice.hosting import JSONRPCServer, ServiceProvider +from ossdbtoolsservice.hosting.json_rpc_server import RequestContext from ossdbtoolsservice.utils import constants from ossdbtoolsservice.connection.contracts import ConnectionDetails, ConnectRequestParams # noqa from ossdbtoolsservice.connection import ConnectionService, ConnectionInfo @@ -17,6 +18,7 @@ ConnectionContext, OperationsQueue, QueuedOperation, INTELLISENSE_URI ) from ossdbtoolsservice.utils.constants import PG_PROVIDER_NAME +from tests.utils import MockRequestContext COMPLETIONREFRESHER_PATH_PATH = 'ossdbtoolsservice.language.operations_queue.CompletionRefresher' @@ -67,13 +69,14 @@ def test_add_context_creates_new_context(self): connect_result.error_message = None self.mock_connection_service.get_connection = mock.Mock(return_value=mock.MagicMock()) self.mock_connection_service.connect = mock.MagicMock(return_value=connect_result) + request_context: RequestContext = MockRequestContext() # When I add a connection context operations_queue = OperationsQueue(self.mock_service_provider) with mock.patch(COMPLETIONREFRESHER_PATH_PATH) as refresher_patch: refresher_patch.return_value = self.refresher_mock - context: ConnectionContext = operations_queue.add_connection_context(self.connection_info) + context: ConnectionContext = operations_queue.add_connection_context(self.connection_info, request_context) # Then I expect the context to be non-null self.assertIsNotNone(context) self.assertEqual(context.key, self.expected_context_key) @@ -84,12 +87,14 @@ def test_add_same_context_twice_creates_one_context(self): def do_test(): # When I add context for 2 URIs with same connection details operations_queue = OperationsQueue(self.mock_service_provider) + request_context_1: RequestContext = MockRequestContext() + request_context_2: RequestContext = MockRequestContext() with mock.patch(COMPLETIONREFRESHER_PATH_PATH) as refresher_patch: refresher_patch.return_value = self.refresher_mock - operations_queue.add_connection_context(self.connection_info) + operations_queue.add_connection_context(self.connection_info, request_context_1) conn_info2 = ConnectionInfo('newuri', self.connection_info.details) - operations_queue.add_connection_context(conn_info2) + operations_queue.add_connection_context(conn_info2, request_context_2) # Then I expect to only have connection connect_mock: mock.MagicMock = self.mock_connection_service.connect connect_mock.assert_called_once() @@ -101,11 +106,13 @@ def test_add_same_context_twice_with_overwrite_creates_two_contexts(self): def do_test(): # When I add context for 2 URIs with same connection details operations_queue = OperationsQueue(self.mock_service_provider) + request_context_1: RequestContext = MockRequestContext() + request_context_2: RequestContext = MockRequestContext() with mock.patch(COMPLETIONREFRESHER_PATH_PATH) as refresher_patch: refresher_patch.return_value = self.refresher_mock - operations_queue.add_connection_context(self.connection_info) + operations_queue.add_connection_context(self.connection_info, request_context_1) conn_info2 = ConnectionInfo('newuri', self.connection_info.details) - operations_queue.add_connection_context(conn_info2, overwrite=True) + operations_queue.add_connection_context(conn_info2, request_context_2, overwrite=True) # Then I expect to only have 1 connection # and I expect disconnect and reconnect to have been called connect_mock: mock.MagicMock = self.mock_connection_service.connect diff --git a/tests/metadata/test_metadata_service.py b/tests/metadata/test_metadata_service.py index ecc307c49..dbd52f792 100644 --- a/tests/metadata/test_metadata_service.py +++ b/tests/metadata/test_metadata_service.py @@ -74,7 +74,7 @@ def test_metadata_list_request(self): self.assertEqual(mock_thread.target, self.metadata_service._metadata_list_worker) mock_thread.start.assert_called_once() # And the worker retrieved the correct connection and executed a query on it - self.connection_service.get_connection.assert_called_once_with(self.test_uri, ConnectionType.DEFAULT) + self.connection_service.get_connection.assert_called_once_with(self.test_uri, ConnectionType.DEFAULT, request_context) mock_cursor.execute.assert_called_once() # And the handler responded with the expected results self.assertIsNone(request_context.last_error_message) @@ -97,7 +97,7 @@ def test_metadata_list_request_error(self): with mock.patch('threading.Thread', new=mock.Mock(side_effect=mock_thread.initialize_target)): # If I call the metadata list request handler and its execution raises an error self.metadata_service._handle_metadata_list_request(request_context, params) - # Then an error response is sent + # Then a telemetry event and error response are sent + self.assertIsNotNone(request_context.last_notification_method) self.assertIsNotNone(request_context.last_error_message) - self.assertIsNone(request_context.last_notification_method) self.assertIsNone(request_context.last_response_params) diff --git a/tests/scripting/test_scripting_service.py b/tests/scripting/test_scripting_service.py index 9957cb11f..86cc79270 100644 --- a/tests/scripting/test_scripting_service.py +++ b/tests/scripting/test_scripting_service.py @@ -11,11 +11,13 @@ from ossdbtoolsservice.connection.contracts import ConnectionCompleteParams from ossdbtoolsservice.hosting import JSONRPCServer, ServiceProvider from ossdbtoolsservice.scripting.contracts.script_as_request import ( - ScriptAsParameters, ScriptAsResponse, ScriptOperation) + SCRIPT_AS_REQUEST, ScriptAsParameters, ScriptAsResponse, ScriptOperation) from ossdbtoolsservice.scripting.scripter import Scripter +from ossdbtoolsservice.utils.telemetry import TELEMETRY_NOTIFICATION, TelemetryParams from ossdbtoolsservice.scripting.scripting_service import ScriptingService from ossdbtoolsservice.utils.constants import (CONNECTION_SERVICE_NAME, PG_PROVIDER_NAME) +from ossdbtoolsservice.exception import constants as error_constants from tests.mock_request_validation import RequestFlowValidator from tests.pgsmo_tests.utils import MockPGServerConnection @@ -61,7 +63,10 @@ def test_handle_scriptas_missing_params(self): ss._service_provider = utils.get_mock_service_provider({}) # If: I make a scripting request missing params - rc: RequestFlowValidator = RequestFlowValidator() + rc: RequestFlowValidator = RequestFlowValidator() + + telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR + rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, lambda param: self._validate_telemetry_error(param, telemetryView, telemetryName, telemetryErrorCode)) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) @@ -80,6 +85,8 @@ def test_handle_scriptas_invalid_operation(self): # If: I create an OE session with missing params rc: RequestFlowValidator = RequestFlowValidator() + telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR + rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, lambda param: self._validate_telemetry_error(param, telemetryView, telemetryName, telemetryErrorCode)) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) @@ -142,3 +149,17 @@ def validate_response(response: ScriptAsResponse) -> None: for calls in matches.values(): self.assertEqual(calls, 1) + + def _validate_telemetry_error(self, telemetryParams: TelemetryParams, view, name, errorCode): + self.assertTrue("eventName" in telemetryParams.params) + self.assertTrue("properties" in telemetryParams.params) + self.assertTrue("measures" in telemetryParams.params) + + properties = telemetryParams.params['properties'] + + self.assertTrue("view" in properties) + self.assertTrue("name" in properties) + self.assertTrue("errorCode" in properties) + self.assertEqual(properties['view'], view) + self.assertEqual(properties['name'], name) + self.assertEqual(properties['errorCode'], errorCode) From 2f637406b3d3e1fc4c06feccfa17d05c9e48bab5 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 16:23:29 -0400 Subject: [PATCH 08/11] tests are passing --- .../query_execution_service.py | 2 +- .../connection/test_pg_connection_service.py | 12 +- tests/integration/config.json | 2 +- tests/mock_request_validation.py | 21 ++++ .../test_object_explorer_service.py | 105 ++++++++++++++++-- tests/scripting/test_scripting_service.py | 18 +-- 6 files changed, 128 insertions(+), 32 deletions(-) diff --git a/ossdbtoolsservice/query_execution/query_execution_service.py b/ossdbtoolsservice/query_execution/query_execution_service.py index 3202e07e7..56809871b 100644 --- a/ossdbtoolsservice/query_execution/query_execution_service.py +++ b/ossdbtoolsservice/query_execution/query_execution_service.py @@ -355,7 +355,7 @@ def _handle_dispose_request(self, request_context: RequestContext, params: Query # If it's not started, then make sure it never starts. If it's executing, make sure # that we stop it if self.query_results[params.owner_uri].execution_state is not ExecutionState.EXECUTED: - self.cancel_query(params.owner_uri) + self.cancel_query(params.owner_uri, request_context) del self.query_results[params.owner_uri] request_context.send_response({}) except Exception as e: diff --git a/tests/connection/test_pg_connection_service.py b/tests/connection/test_pg_connection_service.py index 669e9b393..db1905fa3 100644 --- a/tests/connection/test_pg_connection_service.py +++ b/tests/connection/test_pg_connection_service.py @@ -480,8 +480,10 @@ def test_list_databases_handles_invalid_uri(self): params.owner_uri = 'unknown_uri' self.connection_service.handle_list_databases(mock_request_context, params) - self.assertIsNone(mock_request_context.last_notification_method) - self.assertIsNone(mock_request_context.last_notification_params) + + # Telemetry notification and error. Should be no response + self.assertIsNotNone(mock_request_context.last_notification_method) + self.assertIsNotNone(mock_request_context.last_notification_params) self.assertIsNone(mock_request_context.last_response_params) self.assertIsNotNone(mock_request_context.last_error_message) @@ -507,8 +509,10 @@ def test_list_databases_handles_query_failure(self): with mock.patch('psycopg.connect', new=mock.Mock(return_value=mock_connection)): self.connection_service.handle_list_databases(mock_request_context, params) - self.assertIsNone(mock_request_context.last_notification_method) - self.assertIsNone(mock_request_context.last_notification_params) + + # Telemetry notification and error message. Should be no response + self.assertIsNotNone(mock_request_context.last_notification_method) + self.assertIsNotNone(mock_request_context.last_notification_params) self.assertIsNone(mock_request_context.last_response_params) self.assertIsNotNone(mock_request_context.last_error_message) diff --git a/tests/integration/config.json b/tests/integration/config.json index 9edb59bfd..cd70826dd 100644 --- a/tests/integration/config.json +++ b/tests/integration/config.json @@ -2,6 +2,6 @@ "host": "localhost", "port": 5432, "user": "postgres", - "password": "", + "password": "Boba@123", "dbname": "postgres" } diff --git a/tests/mock_request_validation.py b/tests/mock_request_validation.py index 7036df904..d6dbba02e 100644 --- a/tests/mock_request_validation.py +++ b/tests/mock_request_validation.py @@ -12,6 +12,7 @@ from ossdbtoolsservice.hosting.json_message import JSONRPCMessage, JSONRPCMessageType from ossdbtoolsservice.hosting.json_rpc_server import RequestContext import ossdbtoolsservice.utils as utils +from ossdbtoolsservice.utils.telemetry import TelemetryParams TValidation = TypeVar(Optional[Callable[[any], None]]) @@ -136,6 +137,26 @@ def basic_error_validation(param: ReceivedError) -> None: test_case.assertIsNotNone(param.message) test_case.assertNotEqual(param.message.strip(), '') + @staticmethod + def validate_telemetry_error(view, name, errorCode) -> TValidation: + """Returns single paramter function to validate view, name and errorCode of TelemetryParams for + telemetry error event""" + def validate_telemetry_error_helper(telemetryParams: TelemetryParams): + test_case = unittest.TestCase('__init__') + test_case.assertTrue("eventName" in telemetryParams.params) + test_case.assertTrue("properties" in telemetryParams.params) + test_case.assertTrue("measures" in telemetryParams.params) + + properties = telemetryParams.params['properties'] + + test_case.assertTrue("view" in properties) + test_case.assertTrue("name" in properties) + test_case.assertTrue("errorCode" in properties) + test_case.assertEqual(properties['view'], view) + test_case.assertEqual(properties['name'], name) + test_case.assertEqual(properties['errorCode'], errorCode) + return validate_telemetry_error_helper + # IMPLEMENTATION DETAILS ############################################### def _received_error_callback(self, message: str, data: any = None, code: int = 0): error = ReceivedError(code, message, data) diff --git a/tests/object_explorer/test_object_explorer_service.py b/tests/object_explorer/test_object_explorer_service.py index 908cf2f1c..7a3ffa710 100644 --- a/tests/object_explorer/test_object_explorer_service.py +++ b/tests/object_explorer/test_object_explorer_service.py @@ -10,6 +10,7 @@ import unittest.mock as mock import urllib.parse as url_parse from typing import Callable, Tuple +from ossdbtoolsservice.utils.telemetry import TELEMETRY_NOTIFICATION, TelemetryParams import tests.utils as utils from ossdbtoolsservice.connection import ConnectionService @@ -26,6 +27,7 @@ ObjectExplorerService, ObjectExplorerSession) from ossdbtoolsservice.object_explorer.routing import PG_ROUTING_TABLE from ossdbtoolsservice.utils import constants +from ossdbtoolsservice.exception import constants as error_constants from pgsmo.objects.database.database import Database from pgsmo.objects.server.server import Server from tests.mock_request_validation import RequestFlowValidator @@ -129,7 +131,17 @@ def test_handle_create_session_missing_params(self): oe._service_provider = utils.get_mock_service_provider({}) # If: I create an OE session with missing params - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CREATE_SESSION, + error_constants.OBJECT_EXPLORER_CREATE_SESSION_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) oe._handle_create_session_request(rc.request_context, None) # Then: @@ -147,7 +159,17 @@ def test_handle_create_session_incomplete_params(self): # If: I create an OE session for with missing params # NOTE: We only need to get the generate uri method to throw, we make sure it throws in all # scenarios in a different test - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CREATE_SESSION, + error_constants.OBJECT_EXPLORER_CREATE_SESSION_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) params = ConnectionDetails.from_data({}) oe._handle_create_session_request(rc.request_context, params) @@ -331,12 +353,13 @@ def test_create_connection_successful(self): mock_connection = MockPGServerConnection() oe = ObjectExplorerService() cs = ConnectionService() + request_context: RequestContext = utils.MockRequestContext() cs.connect = mock.MagicMock(return_value=ConnectionCompleteParams()) cs.get_connection = mock.MagicMock(return_value=mock_connection) oe._service_provider = utils.get_mock_service_provider({constants.CONNECTION_SERVICE_NAME: cs}) params, session_uri = _connection_details() session = ObjectExplorerSession(session_uri, params) - connection = oe._create_connection(session, 'foo_database') + connection = oe._create_connection(session, 'foo_database', request_context) self.assertIsNotNone(connection) self.assertEqual(connection, mock_connection) @@ -347,6 +370,7 @@ def test_create_connection_failed(self): # Setup: oe = ObjectExplorerService() cs = ConnectionService() + request_context: RequestContext = utils.MockRequestContext() connect_response = ConnectionCompleteParams() error = 'Failed' connect_response.error_message = error @@ -356,7 +380,7 @@ def test_create_connection_failed(self): session = ObjectExplorerSession(session_uri, params) with self.assertRaises(RuntimeError) as context: - oe._create_connection(session, 'foo_database') + oe._create_connection(session, 'foo_database', request_context) self.assertEqual(error, str(context.exception)) cs.connect.assert_called_once() @@ -441,7 +465,17 @@ def _handle_er_incomplete_params(method: TEventHandler): for params in param_sets: # If: I expand with an invalid set of parameters - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_EXPAND_NODE, + error_constants.OBJECT_EXPLORER_EXPAND_NODE_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) method(oe, rc.request_context, params) # Then: I should get an error response @@ -454,7 +488,17 @@ def _handle_er_no_session_match(method: TEventHandler): oe._service_provider = utils.get_mock_service_provider({}) # If: I expand a node on a session that doesn't exist - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_EXPAND_NODE, + error_constants.OBJECT_EXPLORER_EXPAND_NODE_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) params = ExpandParameters.from_dict({'session_id': 'session', 'node_path': None}) method(oe, rc.request_context, params) @@ -467,7 +511,17 @@ def _handle_er_session_not_ready(self, method: TEventHandler): session.is_ready = False # If: I expand a node on a session that isn't ready - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_EXPAND_NODE, + error_constants.OBJECT_EXPLORER_EXPAND_NODE_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) params = ExpandParameters.from_dict({'session_id': session_uri, 'node_path': None}) method(oe, rc.request_context, params) @@ -656,7 +710,17 @@ def setUp(self): def test_handle_close_session_missing_params(self): # If: I close an OE session with missing params - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) self.oe._handle_close_session_request(rc.request_context, None) # Then: I should get an error response @@ -666,7 +730,17 @@ def test_handle_close_session_incomplete_params(self): # If: I close an OE session for with missing params # NOTE: We only need to get the generate uri method to throw, we make sure it throws in all # scenarios in a different test - rc = RequestFlowValidator().add_expected_error(type(None), RequestFlowValidator.basic_error_validation) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION_ERROR + ) + ) + rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) params = ConnectionDetails.from_data({}) self.oe._handle_close_session_request(rc.request_context, params) @@ -707,7 +781,18 @@ def test_handle_close_session_throwsException(self): self.cs.disconnect = mock.MagicMock(side_effect=Exception) # If: I close an OE session that doesn't exist - rc = RequestFlowValidator().add_expected_error(type(None)) + rc = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.OBJECT_EXPLORER, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION, + error_constants.OBJECT_EXPLORER_CLOSE_SESSION_ERROR + ) + ) + rc.add_expected_error(type(None)) + session_id = _connection_details()[1] params = _close_session_params() params.session_id = session_id diff --git a/tests/scripting/test_scripting_service.py b/tests/scripting/test_scripting_service.py index 86cc79270..c44e8efa0 100644 --- a/tests/scripting/test_scripting_service.py +++ b/tests/scripting/test_scripting_service.py @@ -66,7 +66,7 @@ def test_handle_scriptas_missing_params(self): rc: RequestFlowValidator = RequestFlowValidator() telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR - rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, lambda param: self._validate_telemetry_error(param, telemetryView, telemetryName, telemetryErrorCode)) + rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, RequestFlowValidator.validate_telemetry_error(telemetryView, telemetryName, telemetryErrorCode)) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) @@ -86,7 +86,7 @@ def test_handle_scriptas_invalid_operation(self): # If: I create an OE session with missing params rc: RequestFlowValidator = RequestFlowValidator() telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR - rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, lambda param: self._validate_telemetry_error(param, telemetryView, telemetryName, telemetryErrorCode)) + rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, RequestFlowValidator.validate_telemetry_error(telemetryView, telemetryName, telemetryErrorCode)) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) @@ -149,17 +149,3 @@ def validate_response(response: ScriptAsResponse) -> None: for calls in matches.values(): self.assertEqual(calls, 1) - - def _validate_telemetry_error(self, telemetryParams: TelemetryParams, view, name, errorCode): - self.assertTrue("eventName" in telemetryParams.params) - self.assertTrue("properties" in telemetryParams.params) - self.assertTrue("measures" in telemetryParams.params) - - properties = telemetryParams.params['properties'] - - self.assertTrue("view" in properties) - self.assertTrue("name" in properties) - self.assertTrue("errorCode" in properties) - self.assertEqual(properties['view'], view) - self.assertEqual(properties['name'], name) - self.assertEqual(properties['errorCode'], errorCode) From 15ef04d1743caa4f905fcff88de40dd627903fcb Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Thu, 26 Oct 2023 16:28:42 -0400 Subject: [PATCH 09/11] tests and flake passing --- .../connection/test_pg_connection_service.py | 2 +- tests/scripting/test_scripting_service.py | 27 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/connection/test_pg_connection_service.py b/tests/connection/test_pg_connection_service.py index db1905fa3..ab894ee67 100644 --- a/tests/connection/test_pg_connection_service.py +++ b/tests/connection/test_pg_connection_service.py @@ -509,7 +509,7 @@ def test_list_databases_handles_query_failure(self): with mock.patch('psycopg.connect', new=mock.Mock(return_value=mock_connection)): self.connection_service.handle_list_databases(mock_request_context, params) - + # Telemetry notification and error message. Should be no response self.assertIsNotNone(mock_request_context.last_notification_method) self.assertIsNotNone(mock_request_context.last_notification_params) diff --git a/tests/scripting/test_scripting_service.py b/tests/scripting/test_scripting_service.py index c44e8efa0..939a029a6 100644 --- a/tests/scripting/test_scripting_service.py +++ b/tests/scripting/test_scripting_service.py @@ -11,7 +11,7 @@ from ossdbtoolsservice.connection.contracts import ConnectionCompleteParams from ossdbtoolsservice.hosting import JSONRPCServer, ServiceProvider from ossdbtoolsservice.scripting.contracts.script_as_request import ( - SCRIPT_AS_REQUEST, ScriptAsParameters, ScriptAsResponse, ScriptOperation) + ScriptAsParameters, ScriptAsResponse, ScriptOperation) from ossdbtoolsservice.scripting.scripter import Scripter from ossdbtoolsservice.utils.telemetry import TELEMETRY_NOTIFICATION, TelemetryParams from ossdbtoolsservice.scripting.scripting_service import ScriptingService @@ -63,10 +63,16 @@ def test_handle_scriptas_missing_params(self): ss._service_provider = utils.get_mock_service_provider({}) # If: I make a scripting request missing params - rc: RequestFlowValidator = RequestFlowValidator() - - telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR - rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, RequestFlowValidator.validate_telemetry_error(telemetryView, telemetryName, telemetryErrorCode)) + rc: RequestFlowValidator = RequestFlowValidator() + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.SCRIPTING, + error_constants.SCRIPT_AS_REQUEST, + error_constants.SCRIPTAS_REQUEST_ERROR + ) + ) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) @@ -85,8 +91,15 @@ def test_handle_scriptas_invalid_operation(self): # If: I create an OE session with missing params rc: RequestFlowValidator = RequestFlowValidator() - telemetryView, telemetryName, telemetryErrorCode = error_constants.SCRIPTING, error_constants.SCRIPT_AS_REQUEST, error_constants.SCRIPTAS_REQUEST_ERROR - rc.add_expected_notification(TelemetryParams, TELEMETRY_NOTIFICATION, RequestFlowValidator.validate_telemetry_error(telemetryView, telemetryName, telemetryErrorCode)) + rc.add_expected_notification( + TelemetryParams, + TELEMETRY_NOTIFICATION, + RequestFlowValidator.validate_telemetry_error( + error_constants.SCRIPTING, + error_constants.SCRIPT_AS_REQUEST, + error_constants.SCRIPTAS_REQUEST_ERROR + ) + ) rc.add_expected_error(type(None), RequestFlowValidator.basic_error_validation) ss._handle_script_as_request(rc.request_context, None) From 2c90018247e7a38bbd5ddb4cd35f7c6424511745 Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Wed, 6 Dec 2023 15:47:03 -0500 Subject: [PATCH 10/11] remove pw in test config.json --- tests/integration/config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/config.json b/tests/integration/config.json index cd70826dd..9edb59bfd 100644 --- a/tests/integration/config.json +++ b/tests/integration/config.json @@ -2,6 +2,6 @@ "host": "localhost", "port": 5432, "user": "postgres", - "password": "Boba@123", + "password": "", "dbname": "postgres" } From b9ffb0f3e25a5fed655ca7a16ec646ee393c46dd Mon Sep 17 00:00:00 2001 From: samir-puranik Date: Wed, 13 Dec 2023 16:02:45 -0500 Subject: [PATCH 11/11] empty commit to trigger pipelines