Skip to content

fix: populate overhead duration metrics for all route types - #30607

Open
factnn wants to merge 20 commits into
BerriAI:litellm_internal_stagingfrom
factnn:fix/anthropic-responses-overhead-metrics
Open

fix: populate overhead duration metrics for all route types#30607
factnn wants to merge 20 commits into
BerriAI:litellm_internal_stagingfrom
factnn:fix/anthropic-responses-overhead-metrics

Conversation

@factnn

@factnn factnn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #30566

update_response_metadata was only called from litellm.utils.completion() for
/v1/chat/completions. Routes like /v1/messages (Anthropic) and /v1/responses
(Responses API) skipped this, so x-litellm-overhead-duration-ms and
litellm_overhead_time_ms were never populated.

Fix: call update_response_metadata from base_process_llm_request after the
LLM response is received, covering all route types.

Type

Bug Fix

Changes

  • litellm/proxy/common_request_processing.py: after route_request returns,
    compute overhead via update_response_metadata for all route types

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR attempts to fix missing x-litellm-overhead-duration-ms and litellm_overhead_time_ms metrics for non-chat-completion routes (/v1/messages, /v1/responses) by computing overhead inline in base_process_llm_request after the LLM response is received, guarded by a route-type check to avoid double-writing on the acompletion/completion paths.

  • Overhead computation logic (common_request_processing.py): after route_request returns, overhead is computed from _logging_obj.start_time vs datetime.now() minus llm_api_duration_ms, and written to _hidden_params. The dict-response branch (elif isinstance(response, dict)) writes to a dict key (response[\"_hidden_params\"]), but every downstream reader — including the final header-building call at line 1731 — uses getattr(response, \"_hidden_params\", {}), which ignores dict keys on plain-dict objects. This means the overhead value is still discarded for non-streaming /v1/messages responses.
  • New tests (test_response_metadata.py): test_overhead_computed_for_routes_without_pre_existing_value exercises the pre-existing update_response_metadata function rather than the new inline code path; test_overhead_guard_skips_when_already_present only tests Python's getattr/bool semantics and adds no coverage of the production change.

Confidence Score: 3/5

The dict-response branch writes overhead to a dict key that all downstream getattr readers ignore, leaving the original bug unresolved on the non-streaming /v1/messages path.

The core fix for dict-typed responses (the elif isinstance(response, dict) branch) writes litellm_overhead_time_ms to response["_hidden_params"] as a dict key, but every subsequent reader — lines 1456 and 1731 — calls getattr(response, "_hidden_params", {}), which returns {} for plain dict objects and never sees the key. The overhead metric will still be absent from response headers on the non-streaming Anthropic route.

litellm/proxy/common_request_processing.py — both the seed read and the two downstream getattr calls need a dict-aware branch to make the fix effective for plain-dict responses.

Important Files Changed

Filename Overview
litellm/proxy/common_request_processing.py Adds inline overhead computation for non-chat-completion routes; the dict-response branch writes to a dict key that downstream getattr readers cannot see, leaving /v1/messages overhead still missing from response headers.
tests/test_litellm/litellm_core_utils/llm_response_utils/test_response_metadata.py Adds pytest import and two new test methods; test_overhead_computed_for_routes_without_pre_existing_value exercises update_response_metadata (pre-existing function, not the new production code path), and test_overhead_guard_skips_when_already_present only verifies Python built-in getattr/bool semantics rather than any production logic.

Reviews (14): Last reviewed commit: "chore: re-trigger CI" | Re-trigger Greptile

Comment thread litellm/proxy/common_request_processing.py Outdated
Comment thread litellm/proxy/common_request_processing.py Outdated
@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch 2 times, most recently from 5f99a99 to 8fa63c9 Compare June 17, 2026 04:55
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/common_request_processing.py 92.30% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@factnn

factnn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@akhiljavelin Thanks for approving this PR! I noticed the CI lint failure was due to a scope issue with start_time, so I've pushed a fix for it. Could you take another look when you get a chance? If there are any other CI issues that need fixing, please let me know!

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from d11b7fd to aed9c48 Compare June 17, 2026 07:06
@Sameerlite

Copy link
Copy Markdown
Contributor

Thanks for the PR! A couple of things to get this over the finish line:

  • CI checks are currently failing — could you take a look? If any failures are pre-existing or unrelated to your change, a quick note in a comment helps us move faster.
  • Could you add proof of the change working (screenshots, test output, or a sample request/response)? Even a quick curl before/after really speeds up the review.

Triggering Greptile for a code review in the meantime:

@greptileai

@factnn

factnn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@Sameerlite Hi there, thanks for your review!

Besides the lint failure (missing type_discipline_gate.py — same infrastructure issue as other litellm PRs), there are 5 additional CI failures on this PR. These were already present when @akhiljavelin approved it earlier. I'm currently investigating them, and if you have any guidance on how they were introduced or how to resolve them, I'd really appreciate it.

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from be1098b to 30fe9ff Compare June 18, 2026 10:44
@factnn

factnn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@Sameerlite The lint failure was a missing CI script — the litellm team has now fixed it and it passes. The only remaining failure is proxy-infra, which is pre-existing: it crashes because common_request_processing.py directly accesses logging_obj.litellm_call_id without a None guard at lines 898, 1440, 1629, 1701, 1712, and 2312. None of those lines are touched by this PR. Our update_response_metadata call is guarded by if logging_obj is not None, and internally uses getattr which is None-safe. These same two proxy-infra tests failed when akhiljavelin approved — they were just masked by 5 other test issues I've since resolved. All other tests pass.

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from 5411744 to 1803003 Compare June 18, 2026 13:49
@Sameerlite

Copy link
Copy Markdown
Contributor

Thanks for the update and for explaining the pre-existing failures! A couple of things still needed before we can merge:

  • The proxy-infra / Run tests CI check is still failing. If this is genuinely pre-existing and unrelated to your change, could you link to another recent PR where the same test fails on an unmodified run? That helps us confirm it's infrastructure, not a regression.
  • Could you add proof of the fix working (e.g. test output or a request/response showing x-litellm-overhead-duration-ms is now populated for /v1/messages or /v1/responses)? Even a quick snippet from local test output would help.

Triggering a fresh Greptile review on the updated commit:

@greptileai

Once those are addressed, we'll take another look!

Comment thread litellm/proxy/common_request_processing.py Outdated
@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch 3 times, most recently from 2163f1a to 7aa7b02 Compare June 21, 2026 06:11
@factnn

factnn commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the update and for explaining the pre-existing failures! A couple of things still needed before we can merge:

  • The proxy-infra / Run tests CI check is still failing. If this is genuinely pre-existing and unrelated to your change, could you link to another recent PR where the same test fails on an unmodified run? That helps us confirm it's infrastructure, not a regression.
  • Could you add proof of the fix working (e.g. test output or a request/response showing x-litellm-overhead-duration-ms is now populated for /v1/messages or /v1/responses)? Even a quick snippet from local test output would help.

Triggering a fresh Greptile review on the updated commit:

@greptileai

Once those are addressed, we'll take another look!

@Sameerlite thanks for the review!
截屏2026-06-21 14 04 23
Two updates:

  1. I dug into the proxy-infra failure and found it was actually caused by my change — a variable shadowing issue
    where logging_obj was clobbered inside the new code block.Fixed now, all CI is green with no failures.
  2. The new test
    test_overhead_computed_for_routes_without_pre_existing_value passed in the core-utils / Run tests shard, confirming overhead duration is now set for non-chat-completions routes:
    https://github.com/BerriAI/litellm/actions/runs/27764152259/job/82146155639

@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Thanks for the follow-up and for tracking down the root cause! The fix looks solid:

  • The variable shadowing issue is resolved — _logging_obj now avoids clobbering any outer logging_obj.
  • The guard if not _hidden_params.get("litellm_overhead_time_ms") correctly ensures this is a no-op for routes that already populate overhead (e.g. /v1/chat/completions via the SDK path), so there's no double-counting risk.
  • The new test directly validates the GH#30566 scenario and the math (900ms LLM duration, 1000ms wall time → 100ms overhead) is sound.
  • CI link confirms the new test passes in the core-utils shard.

One minor nit: datetime.now() without tz=timezone.utc can produce a timezone-naive end_time that may interact inconsistently with _logging_obj.start_time if that is timezone-aware. Worth checking whether start_time from Logging objects carries timezone info — if so, datetime.now(tz=timezone.utc) would be safer here to avoid a potential TypeError: can't compare offset-naive and offset-aware datetimes.

Otherwise this looks good to merge once CI is fully green on the latest commit.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

@Sameerlite

Copy link
Copy Markdown
Contributor

@greptileai

@Sameerlite

Copy link
Copy Markdown
Contributor

Thanks for the contribution! A couple of things to get this over the finish line:

  • There are some unresolved Greptile review threads. Could you take a look and address or dismiss those?
  • Could you add some proof that the change works? Screenshots, a sample request/response, test output, or a curl before/after really helps reviewers get confidence.

Once those are addressed, we'll take another look!

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch 2 times, most recently from 88bd035 to c0248d1 Compare June 23, 2026 16:24
Comment thread litellm/proxy/common_request_processing.py
@veria-ai

veria-ai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch 2 times, most recently from b2de52e to fc0e454 Compare June 23, 2026 16:53
@factnn

factnn commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the contribution! A couple of things to get this over the finish line:

  • There are some unresolved Greptile review threads. Could you take a look and address or dismiss those?
  • Could you add some proof that the change works? Screenshots, a sample request/response, test output, or a curl before/after really helps reviewers get confidence.

Once those are addressed, we'll take another look!

@Sameerlite
I tested the Greptile suggestion to use datetime.now(tz=timezone.utc) but it causes TypeError: can't subtract offset-naive and offset-aware datetimes since logging_obj.start_time is timezone-naive — reverted.
The Veria AI issue was caused by a stale base that omitted recent changes to async_streaming_data_generator; rebasing onto the latest litellm_internal_staging resolved it.
Test output / CI proof was already posted in a previous comment with a screenshot.
Let me know if anything else is needed.

@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from fc0e454 to 5eeaa9b Compare June 23, 2026 19:04
chore(ci): promote internal staging to main
@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from cf188a8 to 6959b74 Compare July 2, 2026 12:13
@factnn

factnn commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

chore(ci): promote internal staging to main
@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from bc87138 to 6ecd2df Compare July 4, 2026 07:26
@factnn

factnn commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@Sameerlite is there anything else needed on this one? I've been unable to trigger Greptile re-review via @greptileai as a contributor, would you mind triggering it?

Comment thread litellm/proxy/common_request_processing.py
yuneng-berri and others added 15 commits July 4, 2026 16:12
chore(ci): promote internal staging to main
update_response_metadata was only called in litellm.utils.completion()
for /v1/chat/completions. Routes like /v1/messages (Anthropic) and
/v1/responses (Responses API) skipped this call.

Call update_response_metadata from base_process_llm_request after
the LLM response is received, but only when litellm_overhead_time_ms
is not already set, so chat completions keep their SDK-level timing.

Fixes BerriAI#30566
start_time was not needed for overhead computation on non-chat routes
because llm_api_duration_ms is also not available. Use end_time for
both start and end; the guard prevents affecting chat completions.

Fixes BerriAI#30566
Previously the call passed start_time=end_time, producing
_response_ms=0 and negative litellm_overhead_time_ms.
Use logging_obj.start_time with end_time fallback instead.
Renamed local logging_obj to _logging_obj and start_time to _start_time
to prevent overwriting the function-level logging_obj variable, which
caused proxy-infra test failures when self.data had no litellm_logging_obj.
datetime.now(tz=timezone.utc) is aware but logging_obj.start_time
is naive, causing "can't subtract offset-naive and offset-aware
datetimes" TypeError. Revert to naive datetime.now().
Also restore test file lost in previous API push.
Chat completions already set litellm_overhead_time_ms in the SDK
layer. Skip our overhead calculation for acompletion/completion
routes to avoid interfering with object responses.
Only set litellm_overhead_time_ms in _hidden_params without going
through update_response_metadata which also touches response_cost
and calls the cost calculator.
TypedDict responses (e.g. AnthropicMessagesResponse) are plain
dicts at runtime, so hasattr(response, "_hidden_params") returns
False. Also assign via response["_hidden_params"] for dict types.
@factnn
factnn force-pushed the fix/anthropic-responses-overhead-metrics branch from d12ba5b to 75b049d Compare July 5, 2026 11:20
@factnn

factnn commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

1 similar comment
@factnn

factnn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

@factnn

factnn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

1 similar comment
@factnn

factnn commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread litellm/proxy/common_request_processing.py
@factnn

factnn commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

1 similar comment
@factnn

factnn commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: x-litellm-overhead-duration-ms header and litellm_overhead_time_ms not populated for /v1/messages and /openai/v1/responses endpoints

5 participants