From 1dcf7eb06d6e35907b45482cbd6f2f21b40bea82 Mon Sep 17 00:00:00 2001 From: Anand Taralika <46954145+taralika@users.noreply.github.com> Date: Sun, 12 May 2024 08:47:29 -0700 Subject: [PATCH 1/5] Ignore 0 failures and 0s latency in daily slack reports Should fix #3598 --- litellm/integrations/slack_alerting.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/litellm/integrations/slack_alerting.py b/litellm/integrations/slack_alerting.py index d03922bc1..afa2c67e8 100644 --- a/litellm/integrations/slack_alerting.py +++ b/litellm/integrations/slack_alerting.py @@ -346,8 +346,9 @@ class SlackAlerting(CustomLogger): all_none = True for val in combined_metrics_values: - if val is not None: + if val is not None and val > 0: all_none = False + break if all_none: return False @@ -365,13 +366,14 @@ class SlackAlerting(CustomLogger): for value in failed_request_values ] - ## Get the indices of top 5 keys with the highest numerical values (ignoring None values) + ## Get the indices of top 5 keys with the highest numerical values (ignoring None and 0 values) top_5_failed = sorted( range(len(replaced_failed_values)), key=lambda i: replaced_failed_values[i], reverse=True, )[:5] - + top_5_failed = [index for index in top_5_failed if replaced_failed_values[index] > 0] + # find top 5 slowest # Replace None values with a placeholder value (-1 in this case) placeholder_value = 0 @@ -386,11 +388,14 @@ class SlackAlerting(CustomLogger): key=lambda i: replaced_slowest_values[i], reverse=True, )[:5] + top_5_slowest = [index for index in top_5_slowest if replaced_slowest_values[index] > 0] # format alert -> return the litellm model name + api base message = f"\n\nHere are today's key metrics 📈: \n\n" message += "\n\n*❗️ Top 5 Deployments with Most Failed Requests:*\n\n" + if not top_5_failed: + message += "\tNone\n" for i in range(len(top_5_failed)): key = failed_request_keys[top_5_failed[i]].split(":")[0] _deployment = router.get_model_info(key) @@ -411,6 +416,8 @@ class SlackAlerting(CustomLogger): message += f"\t{i+1}. Deployment: `{deployment_name}`, Failed Requests: `{value}`, API Base: `{api_base}`\n" message += "\n\n*😅 Top 5 Slowest Deployments:*\n\n" + if not top_5_slowest: + message += "\tNone\n" for i in range(len(top_5_slowest)): key = latency_keys[top_5_slowest[i]].split(":")[0] _deployment = router.get_model_info(key) From 9016e29d20fd7dd120a02ff576d197e29db029e1 Mon Sep 17 00:00:00 2001 From: Anand Taralika <46954145+taralika@users.noreply.github.com> Date: Sun, 12 May 2024 08:53:04 -0700 Subject: [PATCH 2/5] Update a comment about ignoring 0 values in addition to None --- litellm/integrations/slack_alerting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litellm/integrations/slack_alerting.py b/litellm/integrations/slack_alerting.py index afa2c67e8..527c874d0 100644 --- a/litellm/integrations/slack_alerting.py +++ b/litellm/integrations/slack_alerting.py @@ -382,7 +382,7 @@ class SlackAlerting(CustomLogger): for value in latency_values ] - # Get the indices of top 5 values with the highest numerical values (ignoring None values) + # Get the indices of top 5 values with the highest numerical values (ignoring None and 0 values) top_5_slowest = sorted( range(len(replaced_slowest_values)), key=lambda i: replaced_slowest_values[i], From b490c289ca3133bbd8a350f253f8f1a0ba392117 Mon Sep 17 00:00:00 2001 From: Anand Taralika <46954145+taralika@users.noreply.github.com> Date: Sun, 12 May 2024 09:29:23 -0700 Subject: [PATCH 3/5] Removed "5" from the string since it's not guaranteed to always be 5, it will be at most 5, but could be less than 5 (if some values are 0s now that we ignore 0s) --- litellm/integrations/slack_alerting.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/litellm/integrations/slack_alerting.py b/litellm/integrations/slack_alerting.py index 527c874d0..788b099ff 100644 --- a/litellm/integrations/slack_alerting.py +++ b/litellm/integrations/slack_alerting.py @@ -393,7 +393,7 @@ class SlackAlerting(CustomLogger): # format alert -> return the litellm model name + api base message = f"\n\nHere are today's key metrics 📈: \n\n" - message += "\n\n*❗️ Top 5 Deployments with Most Failed Requests:*\n\n" + message += "\n\n*❗️ Top Deployments with Most Failed Requests:*\n\n" if not top_5_failed: message += "\tNone\n" for i in range(len(top_5_failed)): @@ -415,7 +415,7 @@ class SlackAlerting(CustomLogger): value = replaced_failed_values[top_5_failed[i]] message += f"\t{i+1}. Deployment: `{deployment_name}`, Failed Requests: `{value}`, API Base: `{api_base}`\n" - message += "\n\n*😅 Top 5 Slowest Deployments:*\n\n" + message += "\n\n*😅 Top Slowest Deployments:*\n\n" if not top_5_slowest: message += "\tNone\n" for i in range(len(top_5_slowest)): From 30332a6d688c7230d2b9f522e608846f804f1789 Mon Sep 17 00:00:00 2001 From: Anand Taralika <46954145+taralika@users.noreply.github.com> Date: Mon, 13 May 2024 21:29:52 -0700 Subject: [PATCH 4/5] Added tests for ignoring 0 metrics when alerting --- litellm/tests/test_alerting.py | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/litellm/tests/test_alerting.py b/litellm/tests/test_alerting.py index b3232cae1..b3518e012 100644 --- a/litellm/tests/test_alerting.py +++ b/litellm/tests/test_alerting.py @@ -359,3 +359,48 @@ async def test_send_llm_exception_to_slack(): ) await asyncio.sleep(3) + + +# test models with 0 metrics are ignored +@pytest.mark.asyncio +async def test_send_daily_reports_ignores_zero_values(): + router = MagicMock() + router.get_model_ids.return_value = ['model1', 'model2', 'model3'] + + slack_alerting = SlackAlerting(internal_usage_cache=MagicMock()) + # model1: failed=None, latency=0; model2: failed=0, latency=0; model3: failed=10, latency=None + slack_alerting.internal_usage_cache.async_batch_get_cache = AsyncMock(return_value=[None, 0, 0, 0, 10, None]) + + router.get_model_info.side_effect = lambda x: {"litellm_params": {"model": x}} + + with patch.object(slack_alerting, 'send_alert', new=AsyncMock()) as mock_send_alert: + result = await slack_alerting.send_daily_reports(router) + + # Check that the send_alert method was called + mock_send_alert.assert_called_once() + message = mock_send_alert.call_args[1]['message'] + + # Ensure the message includes only the non-zero, non-None metrics + assert "model3" in message + assert "model2" not in message + assert "model1" not in message + + assert result == True + + +# test no alert is sent if all None or 0 metrics +@pytest.mark.asyncio +async def test_send_daily_reports_all_zero_or_none(): + router = MagicMock() + router.get_model_ids.return_value = ['model1', 'model2', 'model3'] + + slack_alerting = SlackAlerting(internal_usage_cache=MagicMock()) + slack_alerting.internal_usage_cache.async_batch_get_cache = AsyncMock(return_value=[None, 0, None, 0, None, 0]) + + with patch.object(slack_alerting, 'send_alert', new=AsyncMock()) as mock_send_alert: + result = await slack_alerting.send_daily_reports(router) + + # Check that the send_alert method was not called + mock_send_alert.assert_not_called() + + assert result == False From bd2e4cdfe00811d67bd8c14400f9bfeee9fec72d Mon Sep 17 00:00:00 2001 From: Anand Taralika Date: Mon, 13 May 2024 22:43:12 -0700 Subject: [PATCH 5/5] Fixed the test alert sequence Also fixed the issue that MagicMock does not create asynchronous mocks by default. --- litellm/tests/test_alerting.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/litellm/tests/test_alerting.py b/litellm/tests/test_alerting.py index b3518e012..677061842 100644 --- a/litellm/tests/test_alerting.py +++ b/litellm/tests/test_alerting.py @@ -368,9 +368,10 @@ async def test_send_daily_reports_ignores_zero_values(): router.get_model_ids.return_value = ['model1', 'model2', 'model3'] slack_alerting = SlackAlerting(internal_usage_cache=MagicMock()) - # model1: failed=None, latency=0; model2: failed=0, latency=0; model3: failed=10, latency=None - slack_alerting.internal_usage_cache.async_batch_get_cache = AsyncMock(return_value=[None, 0, 0, 0, 10, None]) - + # model1:failed=None, model2:failed=0, model3:failed=10, model1:latency=0; model2:latency=0; model3:latency=None + slack_alerting.internal_usage_cache.async_batch_get_cache = AsyncMock(return_value=[None, 0, 10, 0, 0, None]) + slack_alerting.internal_usage_cache.async_batch_set_cache = AsyncMock() + router.get_model_info.side_effect = lambda x: {"litellm_params": {"model": x}} with patch.object(slack_alerting, 'send_alert', new=AsyncMock()) as mock_send_alert: