From 9cb5638fbd27efaecb1ea88d5f0cc298c728a711 Mon Sep 17 00:00:00 2001 From: sgoral-splunk <138458044+sgoral-splunk@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:38:46 +0200 Subject: [PATCH] BREAKING CHANGE: add logging function for basic error types (#363) **Issue number:[ADDON-68670](https://splunk.atlassian.net/browse/ADDON-68670)** ## Summary Extending log module with new error logging functions. Breaking change for `log_exception` function. ### Changes * added new functions for error logging. * added `exc_l` parameter to logging message to categorize errors. * added `exc_l` parameter to `log_exception` function as a mandatory argument to log errors with custom category. ### User experience * user can categorize errors in the add-on using new functions. For now 5 basic categories are prepered: Authentication, Permission, Connection, Configuration, Server. * user can add custom error category using `log_exception` function by adding another args `exc_label` * all user's current use of the `log_exception` function must be extended with a `exc_label` argument to categorize error. ## Checklist If your change doesn't seem to apply, please leave them unchecked. * [x] I have performed a self-review of this change * [x] Changes have been tested * [] Changes are documented * [x] PR title follows [conventional commit semantics](https://www.conventionalcommits.org/en/v1.0.0/) --- solnlib/log.py | 53 +++++++++++++++++++++++++++++++++++++++--- tests/unit/test_log.py | 41 ++++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/solnlib/log.py b/solnlib/log.py index 138ea328..3f1958e1 100644 --- a/solnlib/log.py +++ b/solnlib/log.py @@ -20,6 +20,7 @@ import logging.handlers import os.path as op import traceback +from functools import partial from threading import Lock from typing import Dict, Any @@ -251,6 +252,31 @@ def modular_input_end(logger: logging.Logger, modular_input_name: str): ) +def _base_error_log( + logger, + exc: Exception, + exe_label, + full_msg: bool = True, + msg_before: str = None, + msg_after: str = None, +): + log_exception( + logger, + exc, + exc_label=exe_label, + full_msg=full_msg, + msg_before=msg_before, + msg_after=msg_after, + ) + + +log_connection_error = partial(_base_error_log, exe_label="Connection Error") +log_configuration_error = partial(_base_error_log, exe_label="Configuration Error") +log_permission_error = partial(_base_error_log, exe_label="Permission Error") +log_authentication_error = partial(_base_error_log, exe_label="Authentication Error") +log_server_error = partial(_base_error_log, exe_label="Server Error") + + def events_ingested( logger: logging.Logger, modular_input_name: str, @@ -303,12 +329,34 @@ def events_ingested( def log_exception( logger: logging.Logger, e: Exception, + exc_label: str, full_msg: bool = True, msg_before: str = None, msg_after: str = None, log_level: int = logging.ERROR, ): - """General function to log exceptions.""" + """General function to log exceptions. + + Arguments: + logger: Add-on logger. + e: Exception to log. + exc_label: label for the error to categorize it. + full_msg: if set to True, full traceback will be logged. Default: True + msg_before: custom message before exception traceback. Default: None + msg_after: custom message after exception traceback. Default: None + log_level: Log level to log exception. Default: ERROR. + """ + + msg = _get_exception_message(e, full_msg, msg_before, msg_after) + logger.log(log_level, f'exc_l="{exc_label}" {msg}') + + +def _get_exception_message( + e: Exception, + full_msg: bool = True, + msg_before: str = None, + msg_after: str = None, +) -> str: exc_type, exc_value, exc_traceback = type(e), e, e.__traceback__ if full_msg: error = traceback.format_exception(exc_type, exc_value, exc_traceback) @@ -318,5 +366,4 @@ def log_exception( msg_start = msg_before if msg_before is not None else "" msg_mid = "".join(error) msg_end = msg_after if msg_after is not None else "" - msg = f"{msg_start}\n{msg_mid}\n{msg_end}" - logger.log(log_level, msg) + return f"{msg_start}\n{msg_mid}\n{msg_end}" diff --git a/tests/unit/test_log.py b/tests/unit/test_log.py index 2dbc5bc4..aa0298d4 100644 --- a/tests/unit/test_log.py +++ b/tests/unit/test_log.py @@ -247,9 +247,10 @@ def test_log_exceptions_full_msg(): test_jsons = "{'a': 'aa'" json.loads(test_jsons) except Exception as e: - log.log_exception(mock_logger, e, msg_before=start_msg) + log.log_exception(mock_logger, e, "test type1", msg_before=start_msg) mock_logger.log.assert_called_with( - logging.ERROR, f"{start_msg}\n{traceback.format_exc()}\n" + logging.ERROR, + f'exc_l="test type1" {start_msg}\n{traceback.format_exc()}\n', ) @@ -262,10 +263,40 @@ def test_log_exceptions_partial_msg(): json.loads(test_jsons) except Exception as e: log.log_exception( - mock_logger, e, full_msg=False, msg_before=start_msg, msg_after=end_msg + mock_logger, + e, + exc_label="test type", + full_msg=False, + msg_before=start_msg, + msg_after=end_msg, ) mock_logger.log.assert_called_with( logging.ERROR, - "some msg before exception\njson.decoder.JSONDecodeError: Expecting property name enclosed in double " - "quotes: line 1 column 2 (char 1)\n\nsome msg after exception", + 'exc_l="test type" some msg before exception\njson.decoder.JSONDecodeError: Expecting property ' + "name enclosed in double quotes: line 1 column 2 (char 1)\n\nsome msg after exception", + ) + + +@pytest.mark.parametrize( + "func,result", + [ + ("log_connection_error", '"Connection Error"'), + ("log_configuration_error", '"Configuration Error"'), + ("log_permission_error", '"Permission Error"'), + ("log_authentication_error", '"Authentication Error"'), + ("log_server_error", '"Server Error"'), + ], +) +def test_log_basic_error(func, result): + class AddonComplexError(Exception): + pass + + with mock.patch("logging.Logger") as mock_logger: + try: + raise AddonComplexError + except AddonComplexError as e: + fun = getattr(log, func) + fun(mock_logger, e) + mock_logger.log.assert_called_with( + logging.ERROR, f"exc_l={result} \n{traceback.format_exc()}\n" )