From c3184e05c7eb0345c110df7c1364308cf416da44 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Wed, 25 Sep 2024 13:17:49 -0700 Subject: [PATCH 1/4] Remove loguru version checks. --- newrelic/hooks/logger_loguru.py | 21 +++++++++------------ tox.ini | 3 --- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/newrelic/hooks/logger_loguru.py b/newrelic/hooks/logger_loguru.py index e17107f20..bb447b53a 100644 --- a/newrelic/hooks/logger_loguru.py +++ b/newrelic/hooks/logger_loguru.py @@ -69,7 +69,7 @@ def _nr_log_forwarder(message_instance): try: time = record.get("time", None) if time: - time = int(time.timestamp()*1000) + time = int(time.timestamp() * 1000) record_log_event(message, level_name, time, attributes=attrs) except Exception: pass @@ -116,18 +116,15 @@ def _nr_log_patcher(record): record["_nr_original_message"] = message = record["message"] record["message"] = add_nr_linking_metadata(message) - if LOGURU_VERSION > (0, 6, 0): - if original_patcher is not None: - patchers = [p for p in original_patcher] # Consumer iterable into list so we can modify - # Wipe out reference so patchers aren't called twice, as the framework will handle calling other patchers. - original_patcher = None - else: - patchers = [] - - patchers.append(_nr_log_patcher) - return patchers + if original_patcher is not None: + patchers = [p for p in original_patcher] # Consumer iterable into list so we can modify + # Wipe out reference so patchers aren't called twice, as the framework will handle calling other patchers. + original_patcher = None else: - return _nr_log_patcher + patchers = [] + + patchers.append(_nr_log_patcher) + return patchers def wrap_Logger_init(wrapped, instance, args, kwargs): diff --git a/tox.ini b/tox.ini index 3ab7c0562..365e23ab9 100644 --- a/tox.ini +++ b/tox.ini @@ -144,7 +144,6 @@ envlist = python-framework_tornado-{py39,py310,py311,py312}-tornadomaster, python-logger_logging-{py37,py38,py39,py310,py311,py312,pypy310}, python-logger_loguru-{py37,py38,py39,py310,py311,py312,pypy310}-logurulatest, - python-logger_loguru-py39-loguru{06,05}, python-logger_structlog-{py37,py38,py39,py310,py311,py312,pypy310}-structloglatest, python-mlmodel_langchain-{py39,py310,py311,py312}, python-mlmodel_openai-openai0-{py37,py38,py39,py310,py311,py312}, @@ -375,8 +374,6 @@ deps = mlmodel_langchain: mock mlmodel_langchain: asyncio logger_loguru-logurulatest: loguru - logger_loguru-loguru06: loguru<0.7 - logger_loguru-loguru05: loguru<0.6 logger_structlog-structloglatest: structlog messagebroker_pika-pikalatest: pika messagebroker_pika: tornado<5 From 189982718a827956eaf35675f81a0884e8d02e82 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Wed, 25 Sep 2024 13:20:18 -0700 Subject: [PATCH 2/4] Linting in tests. --- tests/logger_loguru/test_local_decorating.py | 15 +++++--- tests/logger_loguru/test_log_forwarding.py | 30 ++++++++++++---- tests/logger_loguru/test_metrics.py | 7 ++-- tests/logger_loguru/test_settings.py | 36 +++++++++++++------- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/tests/logger_loguru/test_local_decorating.py b/tests/logger_loguru/test_local_decorating.py index 5d1273a92..846ab9383 100644 --- a/tests/logger_loguru/test_local_decorating.py +++ b/tests/logger_loguru/test_local_decorating.py @@ -20,7 +20,9 @@ from newrelic.api.transaction import current_transaction from testing_support.fixtures import reset_core_stats_engine from testing_support.validators.validate_log_event_count import validate_log_event_count -from testing_support.validators.validate_log_event_count_outside_transaction import validate_log_event_count_outside_transaction +from testing_support.validators.validate_log_event_count_outside_transaction import ( + validate_log_event_count_outside_transaction, +) def set_trace_ids(): @@ -31,6 +33,7 @@ def set_trace_ids(): if trace: trace.guid = "abcdefgh" + def exercise_logging(logger): set_trace_ids() @@ -42,7 +45,9 @@ def get_metadata_string(log_message, is_txn): assert host entity_guid = application_settings().entity_guid if is_txn: - metadata_string = f"NR-LINKING|{entity_guid}|{host}|abcdefgh12345678|abcdefgh|Python%20Agent%20Test%20%28logger_loguru%29|" + metadata_string = ( + f"NR-LINKING|{entity_guid}|{host}|abcdefgh12345678|abcdefgh|Python%20Agent%20Test%20%28logger_loguru%29|" + ) else: metadata_string = f"NR-LINKING|{entity_guid}|{host}|||Python%20Agent%20Test%20%28logger_loguru%29|" formatted_string = f"{log_message} {metadata_string}" @@ -55,7 +60,7 @@ def test_local_log_decoration_inside_transaction(logger): @background_task() def test(): exercise_logging(logger) - assert logger.caplog.records[0] == get_metadata_string('C', True) + assert logger.caplog.records[0] == get_metadata_string("C", True) test() @@ -65,7 +70,7 @@ def test_local_log_decoration_outside_transaction(logger): @validate_log_event_count_outside_transaction(1) def test(): exercise_logging(logger) - assert logger.caplog.records[0] == get_metadata_string('C', False) + assert logger.caplog.records[0] == get_metadata_string("C", False) test() @@ -80,6 +85,6 @@ def patch(record): def test(): patch_logger = logger.patch(patch) exercise_logging(patch_logger) - assert logger.caplog.records[0] == get_metadata_string('C-PATCH', False) + assert logger.caplog.records[0] == get_metadata_string("C-PATCH", False) test() diff --git a/tests/logger_loguru/test_log_forwarding.py b/tests/logger_loguru/test_log_forwarding.py index aa0414da2..2df50db2a 100644 --- a/tests/logger_loguru/test_log_forwarding.py +++ b/tests/logger_loguru/test_log_forwarding.py @@ -20,7 +20,9 @@ from newrelic.api.transaction import current_transaction from testing_support.fixtures import reset_core_stats_engine from testing_support.validators.validate_log_event_count import validate_log_event_count -from testing_support.validators.validate_log_event_count_outside_transaction import validate_log_event_count_outside_transaction +from testing_support.validators.validate_log_event_count_outside_transaction import ( + validate_log_event_count_outside_transaction, +) from testing_support.validators.validate_log_events import validate_log_events from testing_support.validators.validate_log_events_outside_transaction import validate_log_events_outside_transaction @@ -33,23 +35,33 @@ def set_trace_ids(): if trace: trace.guid = "abcdefgh" + def exercise_logging(logger): set_trace_ids() logger.warning("C") logger.error("D") logger.critical("E") - + assert len(logger.caplog.records) == 3 -_common_attributes_service_linking = {"timestamp": None, "hostname": None, "entity.name": "Python Agent Test (logger_loguru)", "entity.guid": None} -_common_attributes_trace_linking = {"span.id": "abcdefgh", "trace.id": "abcdefgh12345678", **_common_attributes_service_linking} +_common_attributes_service_linking = { + "timestamp": None, + "hostname": None, + "entity.name": "Python Agent Test (logger_loguru)", + "entity.guid": None, +} +_common_attributes_trace_linking = { + "span.id": "abcdefgh", + "trace.id": "abcdefgh12345678", + **_common_attributes_service_linking, +} _test_logging_inside_transaction_events = [ {"message": "C", "level": "WARNING", **_common_attributes_trace_linking}, {"message": "D", "level": "ERROR", **_common_attributes_trace_linking}, - {"message": "E", "level": "CRITICAL", **_common_attributes_trace_linking}, + {"message": "E", "level": "CRITICAL", **_common_attributes_trace_linking}, ] @@ -67,9 +79,10 @@ def test(): _test_logging_outside_transaction_events = [ {"message": "C", "level": "WARNING", **_common_attributes_service_linking}, {"message": "D", "level": "ERROR", **_common_attributes_service_linking}, - {"message": "E", "level": "CRITICAL", **_common_attributes_service_linking}, + {"message": "E", "level": "CRITICAL", **_common_attributes_service_linking}, ] + @reset_core_stats_engine() def test_logging_outside_transaction(logger): @validate_log_events_outside_transaction(_test_logging_outside_transaction_events) @@ -96,6 +109,7 @@ def test(): _test_patcher_application_captured_event = {"message": "C-PATCH", "level": "WARNING"} _test_patcher_application_captured_event.update(_common_attributes_trace_linking) + @reset_core_stats_engine() def test_patcher_application_captured(logger): def patch(record): @@ -112,9 +126,11 @@ def test(): test() + _test_logger_catch_event = {"level": "ERROR"} # Message varies wildly, can't be included in test _test_logger_catch_event.update(_common_attributes_trace_linking) + @reset_core_stats_engine() def test_logger_catch(logger): @validate_log_events([_test_logger_catch_event]) @@ -132,5 +148,5 @@ def throw(): throw() except ValueError: pass - + test() diff --git a/tests/logger_loguru/test_metrics.py b/tests/logger_loguru/test_metrics.py index 9c02d405e..b72a7d2f0 100644 --- a/tests/logger_loguru/test_metrics.py +++ b/tests/logger_loguru/test_metrics.py @@ -14,7 +14,9 @@ from newrelic.api.background_task import background_task from testing_support.fixtures import reset_core_stats_engine -from testing_support.validators.validate_custom_metrics_outside_transaction import validate_custom_metrics_outside_transaction +from testing_support.validators.validate_custom_metrics_outside_transaction import ( + validate_custom_metrics_outside_transaction, +) from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics @@ -22,7 +24,7 @@ def exercise_logging(logger): logger.warning("C") logger.error("D") logger.critical("E") - + assert len(logger.caplog.records) == 3 @@ -33,6 +35,7 @@ def exercise_logging(logger): ("Logging/lines/CRITICAL", 1), ] + @reset_core_stats_engine() def test_logging_metrics_inside_transaction(logger): @validate_transaction_metrics( diff --git a/tests/logger_loguru/test_settings.py b/tests/logger_loguru/test_settings.py index 76bf5a1d0..9d1005630 100644 --- a/tests/logger_loguru/test_settings.py +++ b/tests/logger_loguru/test_settings.py @@ -22,12 +22,15 @@ from testing_support.fixtures import override_application_settings from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics + def get_metadata_string(log_message, is_txn): host = platform.uname().node assert host entity_guid = application_settings().entity_guid if is_txn: - metadata_string = f"NR-LINKING|{entity_guid}|{host}|abcdefgh12345678|abcdefgh|Python%20Agent%20Test%20%28internal_logging%29|" + metadata_string = ( + f"NR-LINKING|{entity_guid}|{host}|abcdefgh12345678|abcdefgh|Python%20Agent%20Test%20%28internal_logging%29|" + ) else: metadata_string = f"NR-LINKING|{entity_guid}|{host}|||Python%20Agent%20Test%20%28internal_logging%29|" formatted_string = f"{log_message} {metadata_string}" @@ -49,10 +52,12 @@ def basic_logging(logger): @pytest.mark.parametrize("feature_setting,subfeature_setting,expected", _settings_matrix) @reset_core_stats_engine() def test_log_forwarding_settings(logger, feature_setting, subfeature_setting, expected): - @override_application_settings({ - "application_logging.enabled": feature_setting, - "application_logging.forwarding.enabled": subfeature_setting, - }) + @override_application_settings( + { + "application_logging.enabled": feature_setting, + "application_logging.forwarding.enabled": subfeature_setting, + } + ) @validate_log_event_count(1 if expected else 0) @background_task() def test(): @@ -65,10 +70,12 @@ def test(): @pytest.mark.parametrize("feature_setting,subfeature_setting,expected", _settings_matrix) @reset_core_stats_engine() def test_local_decorating_settings(logger, feature_setting, subfeature_setting, expected): - @override_application_settings({ - "application_logging.enabled": feature_setting, - "application_logging.local_decorating.enabled": subfeature_setting, - }) + @override_application_settings( + { + "application_logging.enabled": feature_setting, + "application_logging.local_decorating.enabled": subfeature_setting, + } + ) @background_task() def test(): basic_logging(logger) @@ -86,10 +93,13 @@ def test(): @reset_core_stats_engine() def test_log_metrics_settings(logger, feature_setting, subfeature_setting, expected): metric_count = 1 if expected else None - @override_application_settings({ - "application_logging.enabled": feature_setting, - "application_logging.metrics.enabled": subfeature_setting, - }) + + @override_application_settings( + { + "application_logging.enabled": feature_setting, + "application_logging.metrics.enabled": subfeature_setting, + } + ) @validate_transaction_metrics( "test_settings:test_log_metrics_settings..test", custom_metrics=[ From 63ca33bb668b15c91189a88e9a730b7c9e04db7f Mon Sep 17 00:00:00 2001 From: umaannamalai Date: Wed, 25 Sep 2024 20:23:26 +0000 Subject: [PATCH 3/4] [Mega-Linter] Apply linters fixes --- tests/logger_loguru/test_local_decorating.py | 9 +++++---- tests/logger_loguru/test_log_forwarding.py | 13 ++++++++----- tests/logger_loguru/test_metrics.py | 7 +++++-- tests/logger_loguru/test_settings.py | 15 ++++++++++----- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/tests/logger_loguru/test_local_decorating.py b/tests/logger_loguru/test_local_decorating.py index 846ab9383..dc6e3f080 100644 --- a/tests/logger_loguru/test_local_decorating.py +++ b/tests/logger_loguru/test_local_decorating.py @@ -14,16 +14,17 @@ import platform -from newrelic.api.application import application_settings -from newrelic.api.background_task import background_task -from newrelic.api.time_trace import current_trace -from newrelic.api.transaction import current_transaction from testing_support.fixtures import reset_core_stats_engine from testing_support.validators.validate_log_event_count import validate_log_event_count from testing_support.validators.validate_log_event_count_outside_transaction import ( validate_log_event_count_outside_transaction, ) +from newrelic.api.application import application_settings +from newrelic.api.background_task import background_task +from newrelic.api.time_trace import current_trace +from newrelic.api.transaction import current_transaction + def set_trace_ids(): txn = current_transaction() diff --git a/tests/logger_loguru/test_log_forwarding.py b/tests/logger_loguru/test_log_forwarding.py index 2df50db2a..b5644c15b 100644 --- a/tests/logger_loguru/test_log_forwarding.py +++ b/tests/logger_loguru/test_log_forwarding.py @@ -13,18 +13,21 @@ # limitations under the License. import logging -import pytest -from newrelic.api.background_task import background_task -from newrelic.api.time_trace import current_trace -from newrelic.api.transaction import current_transaction +import pytest from testing_support.fixtures import reset_core_stats_engine from testing_support.validators.validate_log_event_count import validate_log_event_count from testing_support.validators.validate_log_event_count_outside_transaction import ( validate_log_event_count_outside_transaction, ) from testing_support.validators.validate_log_events import validate_log_events -from testing_support.validators.validate_log_events_outside_transaction import validate_log_events_outside_transaction +from testing_support.validators.validate_log_events_outside_transaction import ( + validate_log_events_outside_transaction, +) + +from newrelic.api.background_task import background_task +from newrelic.api.time_trace import current_trace +from newrelic.api.transaction import current_transaction def set_trace_ids(): diff --git a/tests/logger_loguru/test_metrics.py b/tests/logger_loguru/test_metrics.py index b72a7d2f0..85f606fcf 100644 --- a/tests/logger_loguru/test_metrics.py +++ b/tests/logger_loguru/test_metrics.py @@ -12,12 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -from newrelic.api.background_task import background_task from testing_support.fixtures import reset_core_stats_engine from testing_support.validators.validate_custom_metrics_outside_transaction import ( validate_custom_metrics_outside_transaction, ) -from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics +from testing_support.validators.validate_transaction_metrics import ( + validate_transaction_metrics, +) + +from newrelic.api.background_task import background_task def exercise_logging(logger): diff --git a/tests/logger_loguru/test_settings.py b/tests/logger_loguru/test_settings.py index 9d1005630..9f13dad33 100644 --- a/tests/logger_loguru/test_settings.py +++ b/tests/logger_loguru/test_settings.py @@ -12,15 +12,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -import pytest import platform +import pytest +from testing_support.fixtures import ( + override_application_settings, + reset_core_stats_engine, +) +from testing_support.validators.validate_log_event_count import validate_log_event_count +from testing_support.validators.validate_transaction_metrics import ( + validate_transaction_metrics, +) + from newrelic.api.application import application_settings from newrelic.api.background_task import background_task -from testing_support.fixtures import reset_core_stats_engine -from testing_support.validators.validate_log_event_count import validate_log_event_count -from testing_support.fixtures import override_application_settings -from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics def get_metadata_string(log_message, is_txn): From 5ddf5f22b8e373b068dbf6a39b9e959c0a92c8bc Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Wed, 25 Sep 2024 14:12:59 -0700 Subject: [PATCH 4/4] Remove unused variable. --- newrelic/hooks/logger_loguru.py | 1 - 1 file changed, 1 deletion(-) diff --git a/newrelic/hooks/logger_loguru.py b/newrelic/hooks/logger_loguru.py index bb447b53a..3c21d4d8a 100644 --- a/newrelic/hooks/logger_loguru.py +++ b/newrelic/hooks/logger_loguru.py @@ -26,7 +26,6 @@ _logger = logging.getLogger(__name__) IS_PYPY = hasattr(sys, "pypy_version_info") -LOGURU_VERSION = get_package_version_tuple("loguru") LOGURU_FILTERED_RECORD_ATTRS = {"extra", "message", "time", "level", "_nr_original_message", "record"} ALLOWED_LOGURU_OPTIONS_LENGTHS = frozenset((8, 9))