Addressing new feedback.

- Proper handling of time to floats. Created a util method and updated code to use it.
- added the missing guard to ensure the app is enabled
This commit is contained in:
Josh Bonczkowski 2026-03-13 13:39:52 -04:00
parent c439ce8257
commit babd6bef77
2 changed files with 43 additions and 16 deletions

View file

@ -331,8 +331,14 @@ class NewRelicLogger(CustomLogger):
return choices[0].get("finish_reason", "unknown")
return "unknown"
def _to_epoch_ms(self, t: Any) -> float:
"""Convert a datetime or float timestamp to epoch milliseconds."""
if hasattr(t, "timestamp"):
return t.timestamp() * 1000.0
return float(t) * 1000.0
def _get_duration(
self, kwargs: Dict, start_time: Optional[float], end_time: Optional[float]
self, kwargs: Dict, start_time: Any, end_time: Any
) -> Optional[float]:
"""
Extract duration in milliseconds.
@ -347,7 +353,7 @@ class NewRelicLogger(CustomLogger):
# Fall back to calculating from timestamps
if start_time is not None and end_time is not None:
return (end_time - start_time) * 1000.0 # Convert to milliseconds
return self._to_epoch_ms(end_time) - self._to_epoch_ms(start_time)
return None
@ -436,11 +442,7 @@ class NewRelicLogger(CustomLogger):
# Add timestamp for request message if available (convert to milliseconds)
if start_time is not None:
# Handle both datetime objects and float timestamps
if hasattr(start_time, "timestamp"):
message_data["timestamp"] = int(start_time.timestamp() * 1000.0)
else:
message_data["timestamp"] = int(start_time * 1000.0)
message_data["timestamp"] = int(self._to_epoch_ms(start_time))
# Only add content if recording is enabled
if self._should_record_content():
@ -465,13 +467,7 @@ class NewRelicLogger(CustomLogger):
# Add timestamp for response message if available (convert to milliseconds)
if end_time is not None:
# Handle both datetime objects and float timestamps
if hasattr(end_time, "timestamp"):
message_data["timestamp"] = int(
end_time.timestamp() * 1000.0
)
else:
message_data["timestamp"] = int(end_time * 1000.0)
message_data["timestamp"] = int(self._to_epoch_ms(end_time))
# Only add content if recording is enabled
if self._should_record_content():
@ -602,7 +598,7 @@ class NewRelicLogger(CustomLogger):
import newrelic.agent
app = newrelic.agent.application()
if app:
if app and app.enabled:
app.record_custom_metric("LLM/LiteLLM/Error", 1)
except Exception as e:
verbose_logger.warning(f"Failed to record New Relic error metric: {e}")

View file

@ -1,5 +1,6 @@
import os
import sys
from datetime import datetime, timezone
from unittest.mock import MagicMock, patch
import pytest
@ -290,6 +291,18 @@ class TestGetFinishReason:
assert self.logger._get_finish_reason({}) == "unknown"
class TestToEpochMs:
def setup_method(self):
self.logger = make_logger()
def test_float_passthrough(self):
assert self.logger._to_epoch_ms(1.0) == pytest.approx(1000.0)
def test_datetime_converted(self):
dt = datetime(2024, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
assert self.logger._to_epoch_ms(dt) == pytest.approx(dt.timestamp() * 1000.0)
class TestGetDuration:
def setup_method(self):
self.logger = make_logger()
@ -298,11 +311,18 @@ class TestGetDuration:
kwargs = {"llm_api_duration_ms": 750.0}
assert self.logger._get_duration(kwargs, 0.0, 1.0) == 750.0
def test_calculates_from_timestamps_when_kwarg_absent(self):
def test_calculates_from_float_timestamps(self):
kwargs = {}
result = self.logger._get_duration(kwargs, 1.0, 2.5)
assert result == pytest.approx(1500.0)
def test_calculates_from_datetime_timestamps(self):
kwargs = {}
start = datetime(2024, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
end = datetime(2024, 1, 1, 0, 0, 1, 500000, tzinfo=timezone.utc) # +1.5s
result = self.logger._get_duration(kwargs, start, end)
assert result == pytest.approx(1500.0)
def test_returns_none_when_nothing_available(self):
assert self.logger._get_duration({}, None, None) is None
@ -398,12 +418,23 @@ class TestRecordErrorMetric:
def test_calls_record_custom_metric(self):
logger = make_logger()
mock_app = MagicMock()
mock_app.enabled = True
with patch("newrelic.agent.application", return_value=mock_app):
logger._record_error_metric()
mock_app.record_custom_metric.assert_called_once_with("LLM/LiteLLM/Error", 1)
def test_skips_when_app_disabled(self):
logger = make_logger()
mock_app = MagicMock()
mock_app.enabled = False
with patch("newrelic.agent.application", return_value=mock_app):
logger._record_error_metric()
mock_app.record_custom_metric.assert_not_called()
# ---------------------------------------------------------------------------
# 9. _emit_supportability_metric