From c439ce8257c766f64e8c8d0669069d9e8ee0f5ca Mon Sep 17 00:00:00 2001 From: Josh Bonczkowski Date: Fri, 13 Mar 2026 13:25:03 -0400 Subject: [PATCH] Addressing additional automated feedback. - Removed a legacy comment about the New Relic header - Reordered imports in one file - Switched another file to use the import at the top of the file instead of inline when used - Added unit tests for untested methods that were identified --- litellm/integrations/newrelic/newrelic.py | 1 - .../custom_logger_registry.py | 2 +- litellm/litellm_core_utils/litellm_logging.py | 5 +- .../integrations/newrelic/test_newrelic.py | 97 +++++++++++++++++++ 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/litellm/integrations/newrelic/newrelic.py b/litellm/integrations/newrelic/newrelic.py index 7eff9b38151..8106465bc3b 100644 --- a/litellm/integrations/newrelic/newrelic.py +++ b/litellm/integrations/newrelic/newrelic.py @@ -234,7 +234,6 @@ class NewRelicLogger(CustomLogger): For the trace ID, we look in kwargs for: - litellm_params.metadata.headers.traceparent (W3C Trace Context) - - litellm_params.metadata.headers.newrelic (New Relic proprietary) If no trace_id is found in headers, generates a random UUID for event grouping. diff --git a/litellm/litellm_core_utils/custom_logger_registry.py b/litellm/litellm_core_utils/custom_logger_registry.py index 75dbbd9fa7a..01bfc6142ca 100644 --- a/litellm/litellm_core_utils/custom_logger_registry.py +++ b/litellm/litellm_core_utils/custom_logger_registry.py @@ -37,11 +37,11 @@ from litellm.integrations.langsmith import LangsmithLogger from litellm.integrations.litellm_agent import LiteLLMAgentModelResolver from litellm.integrations.literal_ai import LiteralAILogger from litellm.integrations.mlflow import MlflowLogger +from litellm.integrations.newrelic import NewRelicLogger from litellm.integrations.openmeter import OpenMeterLogger from litellm.integrations.opentelemetry import OpenTelemetry from litellm.integrations.opik.opik import OpikLogger from litellm.integrations.posthog import PostHogLogger -from litellm.integrations.newrelic import NewRelicLogger from litellm.integrations.prometheus import PrometheusLogger from litellm.integrations.s3_v2 import S3Logger from litellm.integrations.sqs import SQSLogger diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index 156923457dc..b864ecc9aee 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -152,6 +152,7 @@ from ..integrations.litellm_agent import LiteLLMAgentModelResolver from ..integrations.literal_ai import LiteralAILogger from ..integrations.logfire_logger import LogfireLevel, LogfireLogger from ..integrations.lunary import LunaryLogger +from ..integrations.newrelic import NewRelicLogger from ..integrations.openmeter import OpenMeterLogger from ..integrations.opik.opik import OpikLogger from ..integrations.posthog import PostHogLogger @@ -4151,8 +4152,6 @@ def _init_custom_logger_compatible_class( # noqa: PLR0915 _in_memory_loggers.append(gitlab_logger) return gitlab_logger # type: ignore elif logging_integration == "newrelic": - from litellm.integrations.newrelic import NewRelicLogger - for callback in _in_memory_loggers: if isinstance(callback, NewRelicLogger): return callback # type: ignore @@ -4417,8 +4416,6 @@ def get_custom_logger_compatible_class( # noqa: PLR0915 if isinstance(callback, SMTPEmailLogger): return callback elif logging_integration == "newrelic": - from litellm.integrations.newrelic import NewRelicLogger - for callback in _in_memory_loggers: if isinstance(callback, NewRelicLogger): return callback diff --git a/tests/test_litellm/integrations/newrelic/test_newrelic.py b/tests/test_litellm/integrations/newrelic/test_newrelic.py index 2c882083531..dff6d750e9a 100644 --- a/tests/test_litellm/integrations/newrelic/test_newrelic.py +++ b/tests/test_litellm/integrations/newrelic/test_newrelic.py @@ -19,6 +19,7 @@ sys.modules["newrelic.agent"] = _mock_newrelic_agent sys.path.insert(0, os.path.abspath("../..")) +import litellm.integrations.newrelic.newrelic as nr_module from litellm.integrations.newrelic.newrelic import NewRelicLogger @@ -402,3 +403,99 @@ class TestRecordErrorMetric: logger._record_error_metric() mock_app.record_custom_metric.assert_called_once_with("LLM/LiteLLM/Error", 1) + + +# --------------------------------------------------------------------------- +# 9. _emit_supportability_metric +# --------------------------------------------------------------------------- + + +class TestEmitSupportabilityMetric: + def setup_method(self): + self.logger = make_logger() + nr_module._last_metric_emission_time = 0.0 + + def test_records_metric_with_correct_name_and_value(self): + mock_app = MagicMock() + mock_app.enabled = True + with patch("newrelic.agent.application", return_value=mock_app): + with patch.object(self.logger, "_get_litellm_version", return_value="1.80.0"): + self.logger._emit_supportability_metric() + mock_app.record_custom_metric.assert_called_once_with( + "Supportability/Python/ML/LiteLLM/1.80.0", 1 + ) + + def test_updates_last_emission_time(self): + mock_app = MagicMock() + mock_app.enabled = True + fake_now = 9_999_999.0 + with patch("newrelic.agent.application", return_value=mock_app): + with patch("litellm.integrations.newrelic.newrelic.time.time", return_value=fake_now): + self.logger._emit_supportability_metric() + assert nr_module._last_metric_emission_time == fake_now + + def test_skips_when_app_disabled(self): + mock_app = MagicMock() + mock_app.enabled = False + with patch("newrelic.agent.application", return_value=mock_app): + self.logger._emit_supportability_metric() + mock_app.record_custom_metric.assert_not_called() + assert nr_module._last_metric_emission_time == 0.0 + + def test_skips_when_no_app(self): + with patch("newrelic.agent.application", return_value=None): + self.logger._emit_supportability_metric() + assert nr_module._last_metric_emission_time == 0.0 + + +# --------------------------------------------------------------------------- +# 10. _check_and_emit_periodic_metric +# --------------------------------------------------------------------------- + + +class TestCheckAndEmitPeriodicMetric: + def setup_method(self): + self.logger = make_logger() + nr_module._last_metric_emission_time = 0.0 + + def test_emits_on_first_call(self): + """_last_metric_emission_time starts at 0.0; any real time satisfies 27-hour window.""" + with patch.object(self.logger, "_emit_supportability_metric") as mock_emit: + with patch( + "litellm.integrations.newrelic.newrelic.time.time", return_value=100_000.0 + ): + self.logger._check_and_emit_periodic_metric() + mock_emit.assert_called_once() + + def test_does_not_re_emit_within_27_hours(self): + recent = 1_000_000.0 + nr_module._last_metric_emission_time = recent + with patch.object(self.logger, "_emit_supportability_metric") as mock_emit: + with patch( + "litellm.integrations.newrelic.newrelic.time.time", + return_value=recent + 3600, # 1 hour later + ): + self.logger._check_and_emit_periodic_metric() + mock_emit.assert_not_called() + + def test_re_emits_after_27_hours(self): + old = 1_000_000.0 + nr_module._last_metric_emission_time = old + with patch.object(self.logger, "_emit_supportability_metric") as mock_emit: + with patch( + "litellm.integrations.newrelic.newrelic.time.time", + return_value=old + 97201, # 27 hours + 1 second + ): + self.logger._check_and_emit_periodic_metric() + mock_emit.assert_called_once() + + def test_boundary_exactly_27_hours_triggers_emission(self): + old = 1_000_000.0 + nr_module._last_metric_emission_time = old + with patch.object(self.logger, "_emit_supportability_metric") as mock_emit: + with patch( + "litellm.integrations.newrelic.newrelic.time.time", + return_value=old + 97200, + ): + self.logger._check_and_emit_periodic_metric() + mock_emit.assert_called_once()