From 562520bb7fc1b9c2ecde309ff88593bfa85e304d Mon Sep 17 00:00:00 2001 From: oss-agent-shin Date: Mon, 25 May 2026 11:22:48 -0700 Subject: [PATCH] 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. --- litellm/integrations/prometheus.py | 2 +- .../management_endpoints/team_endpoints.py | 19 +++-- .../test_team_endpoints.py | 71 +++++++++++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index 68340f55d99..16cf20fbbb1 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -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"), ) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index c954568bf1e..6b6ad8ba820 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -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, diff --git a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py index f9efbef10bd..3dce98c04a5 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -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