Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove versioned logic in loguru instrumentation #1222

Merged
merged 5 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading