mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-02 02:11:58 +00:00
fix: remove timezone date expansion in daily-activity aggregation
Single-day spend queries from non-UTC timezones over-counted by ~2x because the previous implementation widened the SQL date range by a full UTC day on whichever side the offset pointed. Spend is bucketed in whole-UTC-day rows in LiteLLM_DailyUserSpend, so the expansion pulled an extra 24h of unrelated bucket data per boundary. Concretely on IST (UTC+5:30, offset -330): a single-day query for 2026-05-29 was rewritten to date >= 2026-05-28 AND date <= 2026-05-29 and returned spend across both UTC days. Sums of single-day queries across a 5-day window then exceeded the equivalent multi-day aggregate by ~50%, which is mathematically impossible. Treat the local date range as the UTC date range. The aggregation table has no hour-level granularity, so any conversion using only date arithmetic must round to whole UTC days; the previous fix turned that boundary slop into systematic over-counting. Pass-through trades a small one-time slop at each end of the range for correct, monotonic, additive results across single-day and multi-day queries. Repro from production: bedrock/global.anthropic.claude-opus-4-8 over 2026-05-29 to 2026-06-02, IST timezone: - 5-day aggregate: $701.39 / 1,831 reqs - Sum of 5 single-day queries: $1,070.94 / 2,755 reqs - Excess (was 1.527x): now matches within boundary slop Adds regression tests in TestAdjustDatesForTimezone and TestBuildAggregatedSqlQuery that pin the pass-through behavior and the additivity invariant for any future implementation.
This commit is contained in:
parent
6274b4c217
commit
b495320d87
2 changed files with 140 additions and 30 deletions
|
|
@ -1,5 +1,5 @@
|
|||
import asyncio
|
||||
from datetime import datetime, timedelta
|
||||
from datetime import datetime
|
||||
from types import SimpleNamespace
|
||||
from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
|
||||
|
||||
|
|
@ -386,38 +386,24 @@ def _adjust_dates_for_timezone(
|
|||
timezone_offset_minutes: Optional[int],
|
||||
) -> Tuple[str, str]:
|
||||
"""
|
||||
Adjust date range to account for timezone differences.
|
||||
Pass-through for the local date range; the timezone offset is intentionally ignored here.
|
||||
|
||||
The database stores dates in UTC. When a user in a different timezone
|
||||
selects a local date range, we need to expand the UTC query range to
|
||||
capture all records that fall within their local date range.
|
||||
The aggregation table (e.g. LiteLLM_DailyUserSpend) stores spend in whole-UTC-day
|
||||
buckets keyed on date as YYYY-MM-DD. Any conversion from a local date range to a
|
||||
UTC date range using only date arithmetic must round to whole UTC days, allowing up
|
||||
to 24h of slop at each boundary. The previous implementation expanded the SQL range
|
||||
by an extra full UTC day on whichever side the offset pointed, which pulled in 24h
|
||||
of unrelated bucket data per boundary and produced approximately 100% over-counting
|
||||
on single-day queries (e.g. IST May 29 returning UTC May 28 + UTC May 29 in full).
|
||||
Sums of single-day queries then exceeded the equivalent multi-day aggregate, which
|
||||
is mathematically impossible.
|
||||
|
||||
Args:
|
||||
start_date: Start date in YYYY-MM-DD format (user's local date)
|
||||
end_date: End date in YYYY-MM-DD format (user's local date)
|
||||
timezone_offset_minutes: Minutes behind UTC (positive = west of UTC)
|
||||
This matches JavaScript's Date.getTimezoneOffset() convention.
|
||||
For example: PST = +480 (8 hours * 60 = 480 minutes behind UTC)
|
||||
|
||||
Returns:
|
||||
Tuple of (adjusted_start_date, adjusted_end_date) in YYYY-MM-DD format
|
||||
Treating the local date as the UTC date trades a small one-time boundary slop for
|
||||
correct, monotonic, additive results across single-day and multi-day queries. A
|
||||
later fix can introduce hour-level buckets or pro-rata weighting on adjacent UTC
|
||||
days; both require data the current schema does not store.
|
||||
"""
|
||||
if timezone_offset_minutes is None or timezone_offset_minutes == 0:
|
||||
return start_date, end_date
|
||||
|
||||
start = datetime.strptime(start_date, "%Y-%m-%d")
|
||||
end = datetime.strptime(end_date, "%Y-%m-%d")
|
||||
|
||||
if timezone_offset_minutes > 0:
|
||||
# West of UTC (Americas): local evening extends into next UTC day
|
||||
# e.g., Feb 4 23:59 PST = Feb 5 07:59 UTC
|
||||
end = end + timedelta(days=1)
|
||||
else:
|
||||
# East of UTC (Asia/Europe): local morning starts in previous UTC day
|
||||
# e.g., Feb 4 00:00 IST = Feb 3 18:30 UTC
|
||||
start = start - timedelta(days=1)
|
||||
|
||||
return start.strftime("%Y-%m-%d"), end.strftime("%Y-%m-%d")
|
||||
return start_date, end_date
|
||||
|
||||
|
||||
def _build_where_conditions(
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ sys.path.insert(
|
|||
) # Adds the parent directory to the system path
|
||||
|
||||
from litellm.proxy.management_endpoints.common_daily_activity import (
|
||||
_adjust_dates_for_timezone,
|
||||
_build_aggregated_sql_query,
|
||||
_is_user_agent_tag,
|
||||
get_api_key_metadata,
|
||||
get_daily_activity,
|
||||
|
|
@ -585,3 +587,125 @@ async def test_aggregated_activity_preserves_metadata_for_deleted_keys():
|
|||
assert key_data.metadata.key_alias == "toto-test-2"
|
||||
assert key_data.metadata.team_id == "69cd4b77-b095-4489-8c46-4f2f31d840a2"
|
||||
assert key_data.metrics.spend == 10.0
|
||||
|
||||
|
||||
class TestAdjustDatesForTimezone:
|
||||
"""
|
||||
Regression tests for the timezone double-counting bug.
|
||||
|
||||
Background: the previous implementation expanded the SQL date range by a full
|
||||
UTC day on whichever side a non-UTC timezone offset pointed. Because spend is
|
||||
bucketed in whole UTC days in the aggregation table, that expansion caused
|
||||
single-day queries from non-UTC timezones to include a second full UTC day's
|
||||
worth of data, producing approximately 2x over-counting. The sum of single-day
|
||||
spends across a window then exceeded the equivalent multi-day aggregate, which
|
||||
is mathematically impossible.
|
||||
|
||||
These tests pin the function to a pass-through and assert the additivity
|
||||
invariant that any future implementation must preserve.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"offset_minutes",
|
||||
[
|
||||
None,
|
||||
0,
|
||||
-330, # IST UTC+5:30
|
||||
-540, # JST UTC+9
|
||||
-60, # CET UTC+1
|
||||
240, # AST UTC-4
|
||||
300, # EST UTC-5
|
||||
480, # PST UTC-8
|
||||
],
|
||||
)
|
||||
def test_returns_input_dates_unchanged_for_any_offset(self, offset_minutes):
|
||||
start, end = _adjust_dates_for_timezone(
|
||||
"2026-05-29", "2026-05-29", offset_minutes
|
||||
)
|
||||
assert start == "2026-05-29"
|
||||
assert end == "2026-05-29"
|
||||
|
||||
def test_single_day_query_does_not_widen_to_two_utc_days(self):
|
||||
"""
|
||||
Pins the boundary that caused the original 2x bug: a single IST day must
|
||||
not be translated into a SQL filter covering two UTC days.
|
||||
"""
|
||||
start, end = _adjust_dates_for_timezone("2026-05-29", "2026-05-29", -330)
|
||||
assert start == end == "2026-05-29", (
|
||||
"Single-day IST query expanded to a multi-day UTC range; this is "
|
||||
"the regression that produced approximately 2x over-counting."
|
||||
)
|
||||
|
||||
def test_multi_day_range_endpoints_are_preserved(self):
|
||||
start, end = _adjust_dates_for_timezone("2026-05-29", "2026-06-02", -330)
|
||||
assert (start, end) == ("2026-05-29", "2026-06-02")
|
||||
|
||||
@pytest.mark.parametrize("offset_minutes", [-330, 480])
|
||||
def test_single_day_sums_match_multi_day_window(self, offset_minutes):
|
||||
"""
|
||||
Additivity invariant: querying each day in a window separately and summing
|
||||
the resulting SQL ranges must cover exactly the same range as querying the
|
||||
whole window at once. The bug broke this; without it, single-day sums
|
||||
exceeded the multi-day total by ~50% over a 5-day IST window.
|
||||
"""
|
||||
days = ["2026-05-29", "2026-05-30", "2026-05-31", "2026-06-01", "2026-06-02"]
|
||||
single_day_ranges = [
|
||||
_adjust_dates_for_timezone(d, d, offset_minutes) for d in days
|
||||
]
|
||||
multi_day_range = _adjust_dates_for_timezone(days[0], days[-1], offset_minutes)
|
||||
|
||||
per_day_starts = [r[0] for r in single_day_ranges]
|
||||
per_day_ends = [r[1] for r in single_day_ranges]
|
||||
assert min(per_day_starts) == multi_day_range[0]
|
||||
assert max(per_day_ends) == multi_day_range[1]
|
||||
assert per_day_starts == days
|
||||
assert per_day_ends == days
|
||||
|
||||
|
||||
class TestBuildAggregatedSqlQuery:
|
||||
"""
|
||||
Asserts the SQL emitted by the aggregated query path stays anchored to the
|
||||
user-supplied date range. The original bug shipped a function that returned
|
||||
expanded dates from _adjust_dates_for_timezone, so the regression surface is
|
||||
not just the helper but the SQL it feeds into.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize("offset_minutes", [None, 0, -330, 480])
|
||||
def test_sql_date_bounds_are_user_supplied_dates(self, offset_minutes):
|
||||
sql, params = _build_aggregated_sql_query(
|
||||
table_name="litellm_dailyuserspend",
|
||||
entity_id_field="user_id",
|
||||
entity_id="user-1",
|
||||
start_date="2026-05-29",
|
||||
end_date="2026-05-29",
|
||||
model=None,
|
||||
api_key=None,
|
||||
timezone_offset_minutes=offset_minutes,
|
||||
)
|
||||
|
||||
assert params[0] == "2026-05-29"
|
||||
assert params[1] == "2026-05-29"
|
||||
assert "date >= $1" in sql
|
||||
assert "date <= $2" in sql
|
||||
|
||||
def test_optional_filters_appear_in_params_in_order(self):
|
||||
sql, params = _build_aggregated_sql_query(
|
||||
table_name="litellm_dailyuserspend",
|
||||
entity_id_field="user_id",
|
||||
entity_id="user-1",
|
||||
start_date="2026-05-29",
|
||||
end_date="2026-06-02",
|
||||
model="bedrock/global.anthropic.claude-opus-4-8",
|
||||
api_key="sk-test",
|
||||
timezone_offset_minutes=-330,
|
||||
)
|
||||
|
||||
assert params == [
|
||||
"2026-05-29",
|
||||
"2026-06-02",
|
||||
"user-1",
|
||||
"bedrock/global.anthropic.claude-opus-4-8",
|
||||
"sk-test",
|
||||
]
|
||||
assert "model = $4" in sql
|
||||
assert "api_key = $5" in sql
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue