Support overriding default model provider in instrument_openai by yiphei · Pull Request #1609 · pydantic/logfire

yiphei · 2026-01-07T21:35:17Z

Support overriding the default system provider of openai in instrument_openai to e.g. openrouter. This is necessary to get features like automatic cost computation. For more context: https://pydanticlogfire.slack.com/archives/C06EDRBSAH3/p1767644423267469?thread_ts=1759944637.702099&cid=C06EDRBSAH3

Note: genai_prices.calc_price does not support extraction logic for Openrounter. So the try block that calls genai_prices.calc_price in on_response will error with the following

Traceback (most recent call last):
  File "/Users/yifeiyan/Dev/logfire/logfire/_internal/integrations/llm_providers/openai.py", line 199, in on_response
    usage_data = extract_usage(
        response_data,
        provider_id=model_provider,
        api_flavor='responses' if isinstance(response, Response) else 'chat',
    )
  File "/Users/yifeiyan/Dev/logfire/.venv/lib/python3.13/site-packages/genai_prices/__init__.py", line 94, in extract_usage
    return data_snapshot.get_snapshot().extract_usage(response_data, provider_id, provider_api_url, api_flavor)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yifeiyan/Dev/logfire/.venv/lib/python3.13/site-packages/genai_prices/data_snapshot.py", line 75, in extract_usage
    model_ref, usage = provider.extract_usage(response_data, api_flavor=api_flavor)
                       ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yifeiyan/Dev/logfire/.venv/lib/python3.13/site-packages/genai_prices/types.py", line 315, in extract_usage
    raise ValueError('No extraction logic defined for this provider')
ValueError: No extraction logic defined for this provider

This wont be addressed in this PR because: a) genai_prices is a separate package, so out of scope, and b) client-side computation is not necessary to see the cost info in the UI since that is also computed server-side, though you do lose explicit setting of operation.cost attribute

Run this to check things works as expected

from openai import AsyncOpenAI
import logfire

logfire.configure(
    scrubbing=False,
    console=False,
)
client = AsyncOpenAI(**{
        "base_url": "https://openrouter.ai/api/v1",
        "api_key": <KEY>,
    })
logfire.instrument_openai(client, model_provider='openrouter')

with logfire.span("parent test"):
    await client.chat.completions.create(
        model="gpt-4.1",
        messages=[{"role": "user", "content": "hi, how are you"}],
    )

alexmojaki · 2026-01-08T15:58:06Z

logfire/_internal/integrations/llm_providers/openai.py

        return cast('ResponseT', response)

-    span.set_attribute('gen_ai.system', 'openai')
+    model_provider: str = cast(str, (span.attributes or {}).get('overridden_model_provider', "openai"))


span.attributes won't work when the span isn't recording

@alexmojaki then how should i check if overridden_model_provider is set, or, as you proposed, check that gen_ai.system hasnt already been set upstream

use getattr(span, 'attributes', None)

alexmojaki · 2026-01-08T15:58:41Z

logfire/_internal/integrations/llm_providers/llm_provider.py


            span_data['async'] = is_async
+            if model_provider is not None:
+                span_data['overridden_model_provider'] = model_provider


rather just set gen_ai.system (and gen_ai.provider.name, the new attribute) here

alexmojaki · 2026-01-08T15:59:01Z

logfire/_internal/integrations/llm_providers/llm_provider.py

    get_endpoint_config_fn: Callable[[Any], EndpointConfig],
    on_response_fn: Callable[[Any, LogfireSpan], Any],
    is_async_client_fn: Callable[[type[Any]], bool],
+    model_provider: str | None = None,


just call it provider

alexmojaki · 2026-01-08T16:01:41Z

logfire/_internal/integrations/llm_providers/openai.py

            usage_data = extract_usage(
                response_data,
-                provider_id='openai',
+                provider_id=model_provider,


i think leave this as is, since we're using the openai client we should be able to assume the shape of the usage.

yiphei · 2026-01-08T19:58:19Z

@alexmojaki addressed your feedback. can you quickly review it again before i add tests?

yiphei · 2026-01-08T19:59:32Z

logfire/_internal/integrations/llm_providers/openai.py

        return cast('ResponseT', response)

-    span.set_attribute('gen_ai.system', 'openai')
+    if getattr(span, 'attributes', None) is None or (span.attributes or {}).get('gen_ai.system', None) is None:


@alexmojaki type checking complained that span.attributes could be None, so i had to do (span.attributes or {})

Suggested change

if getattr(span, 'attributes', None) is None or (span.attributes or {}).get('gen_ai.system', None) is None:

if not (getattr(span, 'attributes', None) or {}).get('gen_ai.system'):

doing this will incur the following type checker error

/Users/yifeiyan/Dev/logfire/logfire/_internal/integrations/llm_providers/openai.py:186:12 - error: Type of "get" is partially unknown Type of "get" is "Any | Overload[(key: Unknown, default: None = None, /) -> (Unknown | None), (key: Unknown, default: Unknown, /) -> Unknown, (key: Unknown, default: _T@get, /) -> (Unknown | _T@get)]" (reportUnknownMemberType)

do you want me to suppress it or cast it? the most compact i can make it is

if (getattr(span, 'attributes', {})).get('gen_ai.system', None) is None:

that works too

alexmojaki · 2026-01-08T20:06:19Z

@alexmojaki addressed your feedback. can you quickly review it again before i add tests?

no, please get CI passing first

codecov · 2026-01-08T21:20:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

yiphei · 2026-01-08T21:33:26Z

@alexmojaki addressed your feedback. can you quickly review it again before i add tests?

no, please get CI passing first

@alexmojaki now everything passes except for code coverage and linter that complains about missing docstring for the new param. can you take a quick look now?

alexmojaki · 2026-01-09T09:50:27Z

no, please get CI passing first, and get a review from copilot if possible

yiphei · 2026-01-09T17:13:29Z

no, please get CI passing first, and get a review from copilot if possible

@alexmojaki the reason im nagging you is because this PR contains an API change of a public method (though a backwards compatible one). Usually, orgs are pretty protective about public API changes, so i dont like to do a much of work and then revert because they dislike the API changes. Therefore, all im asking you is a) to quickly confirm that the API change looks good, or b) to tell me that the specific API changes here are no big deal and should proceed without worries

alexmojaki · 2026-01-09T19:04:45Z

Yes it looks fine

yiphei · 2026-01-10T02:56:35Z

@alexmojaki all the tests have been added, and all the CI checks pass. Thanks for the patience.

alexmojaki · 2026-01-12T11:49:53Z

logfire/_internal/integrations/llm_providers/openai.py

        return cast('ResponseT', response)

-    span.set_attribute('gen_ai.system', 'openai')
+    if getattr(span, 'attributes', None) is None or (span.attributes or {}).get('gen_ai.system', None) is None:


Suggested change

if getattr(span, 'attributes', None) is None or (span.attributes or {}).get('gen_ai.system', None) is None:

if not (getattr(span, 'attributes', None) or {}).get('gen_ai.system'):

logfire/_internal/main.py

yiphei · 2026-01-12T17:41:12Z

Note: starting recently, running the test suite locally always fails with the same following errors

..................................................................................................................................................................................... [ 20%]
............................................F                                                                                                                                         [ 25%]tests/otel_integrations/test_sqlalchemy.py:389 test_sqlalchemy_async_instrumentation[engine] - FileNotFoundError: [Errno 2] No such file or directory: 'example2.db'…. [ 25%]
.........................................F                                                                                                                                            [ 29%]tests/test_cli.py:178 test_whoami_no_token_no_url - Failed: DID NOT RAISE <class 'SystemExit'>…. [ 29%]
.................................................................................F                                                                                                    [ 38%]tests/test_configure.py:1362 test_send_to_logfire_if_token_present_empty_via_env_var - assert 1 == 0  +  where 1 = len([<requests_mock.request._RequestObjectProxy object at 0x14…. [ 39%]
.F                                                                                                                                                                                    [ 39%]tests/test_configure.py:1410 test_send_to_logfire_if_token_present_not_empty_via_env_with_empty_arg - AssertionError: assert '' == 'Logfire proj...project_url\n'      - Logfire …. [ 39%]
..E                                                                                                                                                                                   [ 39%]tests/test_configure.py:1362 test_send_to_logfire_if_token_present_empty_via_env_var - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enab…. [ 39%]
.E                                                                                                                                                                                    [ 39%]tests/test_configure.py:1410 test_send_to_logfire_if_token_present_not_empty_via_env_with_empty_arg - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_log…. [ 39%]
.............F                                                                                                                                                                        [ 41%]tests/test_configure.py:1340 test_send_to_logfire_if_token_present - AssertionError: assert 'pylf_v1_us_d9kndG8CDtLQCR6dpg3yNFSX2vbFsx9BcgTlvGLsVzQF' is None  +  where 'pylf_v1_…. [ 41%]
......E                                                                                                                                                                               [ 42%]tests/test_configure.py:1340 test_send_to_logfire_if_token_present - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enable tracemalloc to …. [ 42%]
.....F                                                                                                                                                                                [ 43%]tests/test_configure.py:1349 test_send_to_logfire_if_token_present_empty - assert 1 == 0  +  where 1 = len([<requests_mock.request._RequestObjectProxy object at 0x13d9b6200>])  …. [ 43%]
.......E                                                                                                                                                                              [ 43%]tests/test_configure.py:1349 test_send_to_logfire_if_token_present_empty - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enable tracemall…. [ 44%]
.........F                                                                                                                                                                            [ 45%]tests/test_configure.py:1394 test_send_to_logfire_if_token_present_not_empty - AssertionError: assert '' == 'Logfire proj...project_url\n'      - Logfire project URL: fake_proje…F [ 45%]
                                                                                                                                                                                      [ 45%]tests/test_configure.py:1449 test_configure_unknown_token_region - AssertionError: assert '' == 'Logfire proj...ydantic.dev\n'      - Logfire project URL: https://logfire-us.pyd…. [ 45%]
F                                                                                                                                                                                     [ 45%]tests/test_configure.py:1379 test_send_to_logfire_if_token_present_empty_via_arg - assert 1 == 0  +  where 1 = len([<requests_mock.request._RequestObjectProxy object at 0x147a30…. [ 45%]
......E                                                                                                                                                                               [ 46%]tests/test_configure.py:1394 test_send_to_logfire_if_token_present_not_empty - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enable trace…. [ 46%]
...E                                                                                                                                                                                  [ 46%]tests/test_configure.py:1449 test_configure_unknown_token_region - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enable tracemalloc to ge…. [ 46%]
.E                                                                                                                                                                                    [ 46%]tests/test_configure.py:1379 test_send_to_logfire_if_token_present_empty_via_arg - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread check_logfire_token Enable t…. [ 47%]
..............................F                                                                                                                                                       [ 50%]tests/test_configure.py:589 test_configure_export_delay - ExceptionGroup: multiple thread exception warnings (2 sub-exceptions)…. [ 50%]
..................................................................................................................................................................................... [ 70%]
.....................................................................................s.......................s.......s............................................................... [ 90%]
.x..x....s..s..s.s..s.s.............................................................

I did everything as outlined in https://github.com/pydantic/logfire/blob/main/CONTRIBUTING.md

alexmojaki · 2026-01-12T18:44:26Z

You probably have a .logfire directory in the repo with credentials

logfire/_internal/integrations/llm_providers/openai.py

yiphei · 2026-01-12T19:51:29Z

@alexmojaki you didnt submit a review last time, so re-requesting your review via this comment

Copilot

Pull request overview

This PR adds support for overriding the default model provider in instrument_openai to enable proper attribution and cost calculation for OpenAI-compatible providers like OpenRouter.

Changes:

Added override_provider parameter to instrument_openai() method with comprehensive documentation
Modified instrument_llm_provider() to accept and use override_provider to set the gen_ai.system attribute
Updated on_response() to conditionally set gen_ai.system to 'openai' only when not already present

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`logfire/_internal/main.py`	Added `override_provider` parameter to `instrument_openai()` with documentation explaining its purpose and benefits
`logfire/_internal/integrations/llm_providers/llm_provider.py`	Added `override_provider` parameter and logic to set `gen_ai.system` in span_data when provided
`logfire/_internal/integrations/llm_providers/openai.py`	Modified `on_response()` to only set `gen_ai.system='openai'` when not already present, allowing override
`tests/test_llm_provider.py`	Added parametrized unit tests for sync and async clients with override_provider
`tests/otel_integrations/test_openai.py`	Added comprehensive integration tests covering sync, async, streaming, default behavior, and on_response behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

logfire/_internal/integrations/llm_providers/openai.py

tests/otel_integrations/test_openai.py

alexmojaki · 2026-01-13T14:42:07Z

Please also set PROVIDER_NAME (gen_ai.provider.name) in addition to gen_ai.system, see the just-merged #1619

yiphei · 2026-01-13T18:10:43Z

@alexmojaki done

logfire/_internal/integrations/llm_providers/openai.py

Support custom system pt2

alexmojaki · 2026-01-20T09:49:26Z

logfire/_internal/integrations/llm_providers/openai.py

+            'gen_ai.system': 'openai',
            PROVIDER_NAME: 'openai',


what about the override?

@alexmojaki this happens before the override. this will be overridden

then these lines of code are pointless. looks like PROVIDER_NAME: 'openai', was a mistake in the first place and there was always a better way.

@alexmojaki whats your ask here?

all these pairs of lines 'gen_ai.system': 'openai', PROVIDER_NAME: 'openai', should be removed, they seem to do nothing but add confusion. if tests don't pass without changes after that then it's a sign that overriding isn't working fully.

@alexmojaki sorry, but i no longer have time for this PR. it has expanded way beyond the original scope and what i have time for. do what you want with this PR. I will just fork the repo and implement what i need directly there

added changes

48e858a

yiphei marked this pull request as draft January 7, 2026 21:35

yiphei added 3 commits January 7, 2026 13:37

added changes

5e431dc

added changes

06ce262

fix attribute name

5a2b35d

alexmojaki reviewed Jan 8, 2026

View reviewed changes

alexmojaki mentioned this pull request Jan 8, 2026

OpenRouter has no extractors pydantic/genai-prices#259

Open

yiphei added 3 commits January 8, 2026 11:31

Merge branch 'main' into support-custom-system

85c711f

added changes

8df4eef

added changes

dd9702d

yiphei commented Jan 8, 2026

View reviewed changes

yiphei marked this pull request as ready for review January 8, 2026 20:51

yiphei added 4 commits January 8, 2026 12:54

added changes

33c83d1

added changes

cd656b3

added changes

7f3d01f

added changes

3140257

yiphei added 5 commits January 9, 2026 18:13

Merge branch 'main' into support-custom-system

0e079b2

added changes

1bbd297

added changes

e8cbbaa

added changes

f33ddf3

refactored tests

36bba2f

alexmojaki reviewed Jan 12, 2026

View reviewed changes

yiphei added 2 commits January 12, 2026 09:14

Merge branch 'main' into support-custom-system

fb2d2c0

added changes

3cb7f36

yiphei requested a review from alexmojaki January 12, 2026 17:36

yiphei added 2 commits January 12, 2026 11:42

added changes

d78496d

added changes

8b6085c

yiphei commented Jan 12, 2026

View reviewed changes

logfire/_internal/integrations/llm_providers/openai.py Outdated Show resolved Hide resolved

alexmojaki requested a review from Copilot January 13, 2026 14:30

Copilot started reviewing on behalf of alexmojaki January 13, 2026 14:30 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

logfire/_internal/integrations/llm_providers/openai.py Outdated Show resolved Hide resolved

tests/otel_integrations/test_openai.py Show resolved Hide resolved

yiphei added 5 commits January 13, 2026 09:25

Merge branch 'main' into support-custom-system

190fb88

added changes

a200de6

added changes

fc506ab

added changes

ca42b29

added changes

85186ae

alexmojaki requested changes Jan 14, 2026

View reviewed changes

logfire/_internal/integrations/llm_providers/openai.py Show resolved Hide resolved

logfire/_internal/integrations/llm_providers/openai.py Outdated Show resolved Hide resolved

yiphei requested a review from alexmojaki January 14, 2026 17:29

yiphei added 3 commits January 14, 2026 22:21

Merge branch 'main' into support-custom-system

28e7ee0

added changes

3f9255a

added changes

c702827

yiphei force-pushed the support-custom-system branch from c702827 to 28e7ee0 Compare January 15, 2026 06:39

yiphei mentioned this pull request Jan 15, 2026

Support custom system pt2 yiphei/logfire#7

Merged

yiphei added 2 commits January 19, 2026 16:30

Merge branch 'main' into support-custom-system

c0d1f2d

Merge pull request #7 from yiphei/support-custom-system-pt2

a2aa776

Support custom system pt2

alexmojaki reviewed Jan 20, 2026

View reviewed changes

yiphei requested a review from alexmojaki January 20, 2026 17:10

	if getattr(span, 'attributes', None) is None or (span.attributes or {}).get('gen_ai.system', None) is None:
	if not (getattr(span, 'attributes', None) or {}).get('gen_ai.system'):

Conversation

yiphei commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiphei commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexmojaki commented Jan 8, 2026

Uh oh!

codecov bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yiphei commented Jan 8, 2026

Uh oh!

alexmojaki commented Jan 9, 2026

Uh oh!

yiphei commented Jan 9, 2026

Uh oh!

alexmojaki commented Jan 9, 2026

Uh oh!

yiphei commented Jan 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yiphei commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexmojaki commented Jan 12, 2026

Uh oh!

Uh oh!

yiphei commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

alexmojaki commented Jan 13, 2026

Uh oh!

yiphei commented Jan 13, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

yiphei commented Jan 7, 2026 •

edited

Loading

codecov bot commented Jan 8, 2026 •

edited

Loading

yiphei commented Jan 12, 2026 •

edited

Loading