Skip to content

Commit

Permalink
Merge pull request #1222 from newrelic/remove-loguru-version-check
Browse files Browse the repository at this point in the history
Remove versioned logic in loguru instrumentation
  • Loading branch information
umaannamalai authored Sep 26, 2024
2 parents 4768fba + 5618f78 commit 85f4035
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 56 deletions.
22 changes: 9 additions & 13 deletions newrelic/hooks/logger_loguru.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -69,7 +68,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
Expand Down Expand Up @@ -116,18 +115,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):
Expand Down
20 changes: 13 additions & 7 deletions tests/logger_loguru/test_local_decorating.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@

import platform

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
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


def set_trace_ids():
Expand All @@ -31,6 +34,7 @@ def set_trace_ids():
if trace:
trace.guid = "abcdefgh"


def exercise_logging(logger):
set_trace_ids()

Expand All @@ -42,7 +46,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}"
Expand All @@ -55,7 +61,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()

Expand All @@ -65,7 +71,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()

Expand All @@ -80,6 +86,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()
41 changes: 30 additions & 11 deletions tests/logger_loguru/test_log_forwarding.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@
# limitations under the License.

import logging

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 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 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


def set_trace_ids():
Expand All @@ -33,23 +38,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},
]


Expand All @@ -67,9 +82,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)
Expand All @@ -96,6 +112,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):
Expand All @@ -112,9 +129,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])
Expand All @@ -132,5 +151,5 @@ def throw():
throw()
except ValueError:
pass

test()
14 changes: 10 additions & 4 deletions tests/logger_loguru/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,22 @@
# 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_custom_metrics_outside_transaction import (
validate_custom_metrics_outside_transaction,
)
from testing_support.validators.validate_transaction_metrics import (
validate_transaction_metrics,
)

from newrelic.api.background_task import background_task


def exercise_logging(logger):
logger.warning("C")
logger.error("D")
logger.critical("E")

assert len(logger.caplog.records) == 3


Expand All @@ -33,6 +38,7 @@ def exercise_logging(logger):
("Logging/lines/CRITICAL", 1),
]


@reset_core_stats_engine()
def test_logging_metrics_inside_transaction(logger):
@validate_transaction_metrics(
Expand Down
51 changes: 33 additions & 18 deletions tests/logger_loguru/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,30 @@
# 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):
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}"
Expand All @@ -49,10 +57,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():
Expand All @@ -65,10 +75,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)
Expand All @@ -86,10 +98,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.<locals>.test",
custom_metrics=[
Expand Down
3 changes: 0 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 85f4035

Please sign in to comment.