mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
fix(prometheus): address Greptile review
- Fix bulk-add overcount: emit actual members_after - members_before delta instead of trusting len(data.member), since team_member_add_duplication_check only raises when *every* member is a duplicate (partial-duplicate bulk adds fall through and are silently de-duplicated by _update_team_members_list). - Clarify gauge HELP text: explicitly document that this metric is delta-since-startup rather than absolute current membership (resets to 0 on proxy restart, never reseeded from DB). - Add TestTeamMembersMetricBulkAddDeltaCalculation covering the partial-duplicate bulk-add case and the single-member path.
This commit is contained in:
parent
f5c205db2c
commit
562520bb7f
3 changed files with 87 additions and 5 deletions
|
|
@ -480,7 +480,7 @@ class PrometheusLogger(CustomLogger):
|
|||
# Per-team member count (incremented on add, decremented on remove)
|
||||
self.litellm_team_members_metric = self._gauge_factory(
|
||||
"litellm_team_members_metric",
|
||||
"Current number of members per team. Incremented when a member is added, decremented when a member is removed.",
|
||||
"Net change in team member count since proxy startup, per team. Incremented by 1 on each successful /team/member_add and decremented by 1 on each /team/member_delete. NOTE: this is delta-since-startup, not absolute current membership - the gauge resets to 0 on proxy restart and is never reseeded from the database, so consumers should treat it as a rate-of-change signal rather than a ground-truth count.",
|
||||
labelnames=self.get_labels_for_metric("litellm_team_members_metric"),
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -2491,6 +2491,11 @@ async def team_member_add(
|
|||
prisma_client=prisma_client,
|
||||
)
|
||||
|
||||
# Snapshot pre-add member count so we can compute the real ``+N`` delta for
|
||||
# the Prometheus team-members gauge below (independent of any duplicates
|
||||
# silently skipped by ``_update_team_members_list``).
|
||||
members_before_count = len(complete_team_data.members_with_roles or [])
|
||||
|
||||
(
|
||||
updated_team,
|
||||
updated_users,
|
||||
|
|
@ -2509,10 +2514,16 @@ async def team_member_add(
|
|||
status_code=404, detail={"error": f"Team with id {data.team_id} not found"}
|
||||
)
|
||||
|
||||
# Emit team-members gauge delta (+N) for the just-added members. Duplicates
|
||||
# are rejected earlier by ``team_member_add_duplication_check``, so every
|
||||
# entry in ``data.member`` corresponds to a real net-new membership.
|
||||
members_added = 1 if isinstance(data.member, Member) else len(data.member)
|
||||
# Emit team-members gauge delta for *actual* net-new memberships only.
|
||||
# ``team_member_add_duplication_check`` above only raises when *every*
|
||||
# member in a bulk request is already a duplicate; partial-duplicate bulk
|
||||
# adds fall through and ``_update_team_members_list`` silently skips the
|
||||
# already-present ones. We therefore read the real delta off the
|
||||
# team_members list we just persisted (``complete_team_data.members_with_roles``
|
||||
# is mutated in-place by ``_add_team_members_to_team``) rather than
|
||||
# trusting ``len(data.member)``.
|
||||
members_after_count = len(complete_team_data.members_with_roles or [])
|
||||
members_added = max(0, members_after_count - members_before_count)
|
||||
_emit_team_members_metric_delta(
|
||||
team_id=updated_team.team_id,
|
||||
team_alias=updated_team.team_alias,
|
||||
|
|
|
|||
|
|
@ -8023,3 +8023,74 @@ class TestEmitTeamMembersMetricDelta:
|
|||
team_id="t-1", team_alias="alias-1", amount=1.0
|
||||
)
|
||||
mock_logger.increment_team_members_metric.assert_not_called()
|
||||
|
||||
|
||||
|
||||
class TestTeamMembersMetricBulkAddDeltaCalculation:
|
||||
"""Verify the math used by ``team_member_add`` to compute the gauge delta.
|
||||
|
||||
The endpoint emits ``members_after - members_before`` to the Prometheus
|
||||
team-members gauge so a bulk add with mixed (some duplicate, some new)
|
||||
members increments the gauge only by the count of actually-added members.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_duplicate_bulk_add_does_not_overcount(self):
|
||||
"""3 requested + 1 already-present member => delta is +2, never +3."""
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_TeamTable, Member, TeamMemberAddRequest,
|
||||
)
|
||||
from litellm.proxy.management_endpoints.team_endpoints import (
|
||||
_update_team_members_list,
|
||||
)
|
||||
|
||||
existing_member = Member(
|
||||
user_id="u-existing", user_email="existing@x.com", role="user"
|
||||
)
|
||||
complete_team_data = LiteLLM_TeamTable(
|
||||
team_id="t-partial-dup", team_alias="partial-dup-alias",
|
||||
members_with_roles=[existing_member],
|
||||
)
|
||||
data = TeamMemberAddRequest(
|
||||
team_id="t-partial-dup",
|
||||
member=[
|
||||
Member(user_id="u-existing", user_email="existing@x.com", role="user"),
|
||||
Member(user_id="u-new-a", user_email="new-a@x.com", role="user"),
|
||||
Member(user_id="u-new-b", user_email="new-b@x.com", role="user"),
|
||||
],
|
||||
)
|
||||
members_before_count = len(complete_team_data.members_with_roles or [])
|
||||
await _update_team_members_list(
|
||||
data=data, complete_team_data=complete_team_data, updated_users=[],
|
||||
)
|
||||
members_after_count = len(complete_team_data.members_with_roles or [])
|
||||
delta = max(0, members_after_count - members_before_count)
|
||||
assert members_before_count == 1
|
||||
assert members_after_count == 3
|
||||
assert delta == 2, (
|
||||
f"Expected gauge delta of +2 (only the two new members), "
|
||||
f"got +{delta}. ``len(data.member)`` would have wrongly given +3."
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_single_member_add_delta_is_one(self):
|
||||
"""The single-member (non-bulk) path emits exactly +1."""
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_TeamTable, Member, TeamMemberAddRequest,
|
||||
)
|
||||
from litellm.proxy.management_endpoints.team_endpoints import (
|
||||
_update_team_members_list,
|
||||
)
|
||||
complete_team_data = LiteLLM_TeamTable(
|
||||
team_id="t-single", team_alias="single-alias", members_with_roles=[],
|
||||
)
|
||||
data = TeamMemberAddRequest(
|
||||
team_id="t-single",
|
||||
member=Member(user_id="u-only", user_email="only@x.com", role="user"),
|
||||
)
|
||||
members_before_count = len(complete_team_data.members_with_roles or [])
|
||||
await _update_team_members_list(
|
||||
data=data, complete_team_data=complete_team_data, updated_users=[],
|
||||
)
|
||||
members_after_count = len(complete_team_data.members_with_roles or [])
|
||||
assert max(0, members_after_count - members_before_count) == 1
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue