Skip to content

Commit 3ba999c

Browse files
fix(obs): route GenAI metrics on litellm's provider, not the model prefix
Review finding from @harvhan on #523, confirmed against litellm 1.87.0 before fixing. Reading the vendor off the model prefix was wrong in two opposite directions, and both were silent. **Azure was counted twice.** `azure/gpt-4o` looks like a native vendor to a prefix reader, so this recorded it — but litellm serves azure with `openai.AzureOpenAI` (litellm/main.py, `if custom_llm_provider == "azure"`), so the OpenAI client instrumentor recorded it as well. The same held for every openai-compatible provider litellm supports: groq, deepseek, xai, fireworks_ai and ~50 others all carry a vendor prefix and all leave over the openai client. **Bare Anthropic models were not counted at all.** `claude-sonnet-4-20250514` is a legal litellm model string that resolves to provider `anthropic` and routes natively, but an unprefixed name was assumed to be OpenAI, so this stood down for an instrumentor that never saw the call. The provider now comes from `litellm.get_llm_provider` — the same resolution litellm uses to route — and the openai-client question from litellm's own `openai_compatible_providers` rather than a list of ours, since that list grows every release. Azure and four siblings are added explicitly; litellm dispatches them over the same client without listing them there. model before after azure/gpt-4o (azure, False) (azure, True) groq/llama3-8b-8192 (groq, False) (groq, True) deepseek/deepseek-chat (deepseek, False) (deepseek, True) claude-sonnet-4-20250514 (openai, True) (anthropic, False) anthropic/claude-sonnet-4 unchanged bedrock/anthropic.claude-v2 unchanged litellm_proxy/anthropic/claude-sonnet-4 unchanged The proxy prefix is still stripped before resolving, so a proxied call keeps reporting the vendor underneath — the one thing the OpenAI client instrumentor cannot report, and the reason this module exists. Resolution is cached behind a bounded dict. Beyond speed (1.9us -> 0.09us), it is what stops litellm's red "Provider List" banner — printed to STDOUT, not through logging, so it cannot be filtered — appearing on every call for a model litellm cannot place. Measured: 50 calls print it once, not 50 times. Redirecting stdout around the lookup was the alternative and is worse; it swaps a process-global, so under concurrency it would swallow other coroutines' output. A plain dict rather than lru_cache because `functools.lru_cache` is banned here (TID251) and the sanctioned one lives in `agentex._utils`, which `agentex/lib` does not otherwise import from. None of this touches agents without sgp-obs: `inference_call` returns the null recorder before `_split_model` is ever reached (verified). Gateway cost on that path, same harness back to back: 13.1us on 0.26.0, 11.9us here. Tests: 33 in this file (was 22), 166 across the obs suites, 1714 repo-wide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 00b738d commit 3ba999c

2 files changed

Lines changed: 192 additions & 9 deletions

File tree

‎src/agentex/lib/core/adapters/llm/_genai_metrics.py‎

Lines changed: 115 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,39 @@
4040
# A bare model name with no "<vendor>/" prefix is OpenAI, per litellm's own default.
4141
_DEFAULT_VENDOR = "openai"
4242

43+
# Providers litellm dispatches through the `openai` Python client, and which the OpenAI
44+
# client instrumentor therefore already records, but which litellm does NOT carry in
45+
# `openai_compatible_providers`. Azure is the one that matters: it is served by
46+
# openai.AzureOpenAI (litellm/main.py, `if custom_llm_provider == "azure"`), so reading
47+
# the prefix alone and calling it a native vendor double-counted every Azure call.
48+
_EXTRA_OPENAI_CLIENT_PROVIDERS = frozenset(
49+
{"openai", "azure", "azure_text", "text-completion-openai", "custom_openai"}
50+
)
51+
52+
_OPENAI_CLIENT_PROVIDERS_UNRESOLVED = object()
53+
_openai_client_providers: Any = _OPENAI_CLIENT_PROVIDERS_UNRESOLVED
54+
55+
56+
def _over_openai_client(provider: str) -> bool:
57+
"""Would the OpenAI client instrumentor already have recorded this call?
58+
59+
Answered from litellm's own ``openai_compatible_providers`` rather than a list of
60+
our own, because that list is what litellm actually routes on and it grows every
61+
release (54 entries as of 1.87.0: groq, deepseek, xai, fireworks_ai, ...).
62+
"""
63+
global _openai_client_providers
64+
if _openai_client_providers is _OPENAI_CLIENT_PROVIDERS_UNRESOLVED:
65+
try:
66+
import litellm
67+
68+
_openai_client_providers = (
69+
frozenset(litellm.openai_compatible_providers)
70+
| _EXTRA_OPENAI_CLIENT_PROVIDERS
71+
)
72+
except Exception: # pragma: no cover - litellm is a hard dependency
73+
_openai_client_providers = _EXTRA_OPENAI_CLIENT_PROVIDERS
74+
return provider in _openai_client_providers
75+
4376
# sgp-obs is an optional install (see ``sgp_obs_setup``), and Python does NOT cache a
4477
# FAILED import, so importing inside ``inference_call`` re-walked sys.path on every
4578
# single model call for the majority of agents that do not have it. Measured on
@@ -76,20 +109,93 @@ def _genai() -> Any | None:
76109

77110

78111
def _split_model(model: str) -> tuple[str, bool]:
79-
"""``(vendor, goes_out_over_the_openai_client)`` for a litellm model string.
112+
"""``(provider, goes_out_over_the_openai_client)`` for a litellm model string.
80113
81114
``"litellm_proxy/anthropic/claude-sonnet-4"`` -> ``("anthropic", True)``
82115
``"anthropic/claude-sonnet-4"`` -> ``("anthropic", False)``
116+
``"claude-sonnet-4-20250514"`` -> ``("anthropic", False)``
117+
``"azure/gpt-4o"`` -> ``("azure", True)``
83118
``"gpt-4o"`` -> ``("openai", True)``
84119
85-
A bare name is OpenAI, and litellm reaches OpenAI through the ``openai``
86-
client, so the client instrumentor already sees it and we stand down.
120+
The provider comes from ``litellm.get_llm_provider`` — the same resolution litellm
121+
uses to route the call — rather than from reading the prefix. Reading the prefix got
122+
two whole classes of call wrong, in opposite directions:
123+
124+
* **Prefixed but still over the OpenAI client.** ``azure/gpt-4o`` looks like a
125+
native vendor, but litellm serves it with ``openai.AzureOpenAI``, so the client
126+
instrumentor recorded it too and this recorded it a second time. The same held
127+
for every openai-compatible provider litellm supports — groq, deepseek, xai,
128+
fireworks_ai and ~50 others — all of which look "native" to a prefix reader.
129+
* **Unprefixed but NOT OpenAI.** ``claude-sonnet-4-20250514`` is a legal litellm
130+
model string that routes to Anthropic, but a bare name was assumed to be OpenAI,
131+
so this stood down for an instrumentor that never saw the call. Nothing recorded
132+
it and nothing said so.
133+
134+
The proxy prefix is stripped before resolving, deliberately: ``litellm_proxy/`` is a
135+
routing instruction, so the vendor underneath it is the interesting label — and the
136+
one thing the OpenAI client instrumentor cannot report, since from inside that
137+
client the call is simply "openai".
87138
"""
88139
proxied = model.startswith(_PROXY_PREFIX)
89140
rest = model[len(_PROXY_PREFIX):] if proxied else model
90-
vendor = rest.split("/", 1)[0] if "/" in rest else _DEFAULT_VENDOR
91-
# Proxy mode always leaves over the OpenAI client. So does a native openai/* call.
92-
return (vendor or _DEFAULT_VENDOR), proxied or vendor == _DEFAULT_VENDOR
141+
142+
provider = _resolve_provider(rest)
143+
if provider is None:
144+
# litellm could not resolve it, which means it would not route the call either.
145+
# Fall back to the prefix so an exotic string still gets a sensible label.
146+
provider = rest.split("/", 1)[0] if "/" in rest else _DEFAULT_VENDOR
147+
provider = provider or _DEFAULT_VENDOR
148+
149+
# Proxy mode always leaves over the OpenAI client, whatever the vendor underneath.
150+
return provider, proxied or _over_openai_client(provider)
151+
152+
153+
# Resolved providers, keyed by model string. A plain dict rather than lru_cache:
154+
# `functools.lru_cache` is banned in this repo (TID251) and the sanctioned replacement
155+
# lives in `agentex._utils`, which is the generated client half that `agentex/lib` does
156+
# not otherwise import from. This module already keeps two other resolve-once caches,
157+
# so a third is the least surprising option.
158+
#
159+
# Bounded because the key is a model string, and a fine-tune id or a caller building
160+
# names dynamically would otherwise grow it without limit. An agent talks to a handful
161+
# of models, so the cap is never reached in practice; clearing wholesale when it is
162+
# keeps the bookkeeping to nothing.
163+
_PROVIDER_CACHE_MAX = 256
164+
_provider_cache: dict[str, str | None] = {}
165+
166+
167+
def _resolve_provider(model: str) -> str | None:
168+
"""litellm's own provider for ``model``, or None when it cannot resolve one.
169+
170+
``get_llm_provider`` raises ``BadRequestError`` for a model it does not know
171+
(measured: ``claude-3-5-sonnet-latest`` raises, ``claude-sonnet-4-20250514`` does
172+
not), and a telemetry lookup must never be the reason a model call fails.
173+
174+
Cached for two reasons beyond speed. litellm prints a red "Provider List: ..."
175+
banner to STDOUT when resolution fails — not through logging, so it cannot be
176+
filtered — and uncached, an agent on a model string litellm cannot place would
177+
print it on every single call. Redirecting stdout around the lookup was the
178+
alternative and is worse: it swaps a process-global for the duration, so under
179+
concurrency it would swallow output belonging to other coroutines.
180+
"""
181+
if not model:
182+
return None
183+
if model in _provider_cache:
184+
return _provider_cache[model]
185+
186+
provider: str | None = None
187+
try:
188+
from litellm import get_llm_provider
189+
190+
_model, resolved, _key, _base = get_llm_provider(model=model)
191+
provider = resolved or None
192+
except Exception:
193+
provider = None
194+
195+
if len(_provider_cache) >= _PROVIDER_CACHE_MAX:
196+
_provider_cache.clear()
197+
_provider_cache[model] = provider
198+
return provider
93199

94200

95201
def resolve_model(args: tuple[Any, ...], kwargs: dict[str, Any]) -> str:
@@ -165,5 +271,7 @@ def _reset_for_tests() -> None:
165271
sgp-obs absent would cache None for the rest of the session and every later test
166272
that injects a fake ``sgp_obs.metrics`` would silently exercise the null path.
167273
"""
168-
global _genai_module
274+
global _genai_module, _openai_client_providers
169275
_genai_module = _GENAI_UNRESOLVED
276+
_openai_client_providers = _OPENAI_CLIENT_PROVIDERS_UNRESOLVED
277+
_provider_cache.clear()

‎src/agentex/lib/core/adapters/llm/tests/test_genai_metrics.py‎

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,96 @@ class TestSplitModel:
5353
("anthropic/claude-sonnet-4", "anthropic", False),
5454
("bedrock/anthropic.claude-v2", "bedrock", False),
5555
("vertex_ai/gemini-2.0-flash", "vertex_ai", False),
56-
# A bare name is OpenAI per litellm's default, and reaches OpenAI
57-
# through the openai client — so the instrumentor already sees it.
56+
# A bare name litellm cannot place is OpenAI per its own default, and
57+
# reaches OpenAI through the openai client — the instrumentor sees it.
5858
("gpt-4o", "openai", True),
5959
("openai/gpt-4o", "openai", True),
60+
# Azure is served by openai.AzureOpenAI, so the client instrumentor
61+
# records it and we must NOT. Reading the prefix called this native.
62+
("azure/gpt-4o", "azure", True),
63+
# Every openai-compatible provider litellm supports has the same shape:
64+
# a vendor prefix, but dispatched over the openai client.
65+
("groq/llama3-8b-8192", "groq", True),
66+
("deepseek/deepseek-chat", "deepseek", True),
67+
# A bare Anthropic model is legal and routes NATIVELY to Anthropic, so
68+
# nothing else records it. The prefix reader called this OpenAI and
69+
# stood down for an instrumentor that never saw the call.
70+
("claude-sonnet-4-20250514", "anthropic", False),
71+
# Genuinely native: no openai client anywhere in the path.
72+
("gemini/gemini-2.0-flash", "gemini", False),
6073
],
6174
)
6275
def test_vendor_and_transport(self, model, vendor, over_openai_client):
6376
assert _split_model(model) == (vendor, over_openai_client)
6477

78+
def test_an_unresolvable_model_falls_back_to_the_prefix(self):
79+
"""litellm raises for a model it cannot place (measured:
80+
claude-3-5-sonnet-latest). That call will fail in litellm too, but the lookup
81+
must not raise on the way there."""
82+
assert _split_model("claude-3-5-sonnet-latest") == ("openai", True)
83+
assert _split_model("madeup_vendor/some-model") == ("madeup_vendor", False)
84+
6585
def test_empty_model_does_not_raise(self):
6686
"""kwargs.get("model") is "" when a caller passes model positionally.
6787
Falling back to litellm's own default is right, and must not blow up."""
6888
assert _split_model("") == ("openai", True)
6989

7090

91+
class TestTheRoutingDecisionComesFromLitellm:
92+
"""The boolean decides whether `call()` stands down for the OpenAI client
93+
instrumentor or records itself, so getting it wrong either double-counts a call or
94+
loses it entirely. Both happened while it was read off the model prefix."""
95+
96+
def test_azure_is_not_double_counted(self):
97+
"""litellm serves azure/* with openai.AzureOpenAI (litellm/main.py,
98+
`if custom_llm_provider == "azure"`), so the client instrumentor already
99+
records it. Recording here as well counted every Azure call twice."""
100+
_provider, over_openai_client = _split_model("azure/gpt-4o")
101+
assert over_openai_client is True
102+
103+
def test_a_bare_anthropic_model_is_recorded(self):
104+
"""The opposite failure: nothing else sees this call, so standing down meant
105+
it went unmeasured and nothing said so."""
106+
provider, over_openai_client = _split_model("claude-sonnet-4-20250514")
107+
assert provider == "anthropic"
108+
assert over_openai_client is False
109+
110+
def test_the_proxy_vendor_survives_resolution(self):
111+
"""The reason this module exists at all: from inside the OpenAI client a
112+
proxied call is just "openai". The prefix is stripped before resolving so the
113+
vendor underneath is still the label."""
114+
assert _split_model("litellm_proxy/anthropic/claude-sonnet-4") == (
115+
"anthropic",
116+
True,
117+
)
118+
119+
def test_the_provider_list_agrees_with_litellm(self):
120+
"""Pinned to litellm's own list rather than a copy of it, because the copy
121+
would go stale every release."""
122+
import litellm
123+
124+
from agentex.lib.core.adapters.llm._genai_metrics import _over_openai_client
125+
126+
for provider in list(litellm.openai_compatible_providers)[:20]:
127+
assert _over_openai_client(provider), provider
128+
for provider in ("anthropic", "bedrock", "vertex_ai", "gemini"):
129+
assert not _over_openai_client(provider), provider
130+
131+
def test_an_unresolvable_model_does_not_spam_stdout(self):
132+
"""litellm prints a red "Provider List" banner to STDOUT (not logging, so it
133+
cannot be filtered) every time resolution fails. Uncached, an agent on a model
134+
string litellm cannot place printed it on every single call."""
135+
import io
136+
import contextlib
137+
138+
_genai_metrics._reset_for_tests()
139+
buf = io.StringIO()
140+
with contextlib.redirect_stdout(buf):
141+
for _ in range(25):
142+
_split_model("claude-3-5-sonnet-latest")
143+
assert buf.getvalue().count("Provider List") <= 1, buf.getvalue()[:400]
144+
145+
71146
class TestFailsOpenWithoutSgpObs:
72147
@staticmethod
73148
def _hide_sgp_obs(monkeypatch):

0 commit comments

Comments
 (0)