Conversation
get_server_address_and_port checked isinstance(base_url, httpx.URL), but openai 3.x builds on httpx2, so base_url is an httpx2.URL. Neither that branch nor the str branch matched, the function returned (None, None), and server.address and server.port were dropped from every span with no warning. Reading the URL structurally with getattr covers httpx.URL, httpx2.URL, any other URL-like object and plain strings, so the extraction no longer depends on which HTTP client the SDK uses internally. Existing behaviour is preserved on every path that worked before, including omitting the default 443 port. Removing the isinstance check also removes the only use of the httpx import. That matters on its own: this package declares no dependency on httpx while openai 3.x depends on httpx2, so on a clean install httpx can be absent and the module-scope import made the package unimportable rather than merely degraded. Tests cover httpx.URL, httpx2.URL, a plain string, an explicit :443 (asserting the port is still omitted), a missing base_url, and that the package imports with httpx uninstalled.
|
Hi @ari-abb can thanks for the PR, but development on this has moved to https://github.com/open-telemetry/opentelemetry-python-genai (see #4924). The version in this repo will be deleted from this repo once we make a final release with the deprecation notice. I'm happy to transfer the issue over if you can reopen the PR? |
Pull request dashboard statusWaiting on the author · refreshed 2026-09-17 08:39 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
|
Hi @ari-abb — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.
|
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
This branch has not been deployed
3 participants
Fixes #4953
Description
get_server_address_and_portdecided how to readbase_urlwithisinstance(base_url, httpx.URL).openai3.x builds onhttpx2, soclient._client.base_urlis anhttpx2.URL: neither that branch nor thestrbranch matches, the function returns(None, None), andserver.addressandserver.portare silently dropped from every span.This reads the URL structurally instead, which covers
httpx.URL,httpx2.URL, any other URL-likeobject and plain strings, so extraction no longer depends on which HTTP client the SDK uses internally.
Removing the
isinstancecheck also removes the only use of thehttpximport, which mattersseparately: this package declares no dependency on
httpx, andopenai3.x depends onhttpx2instead, so on a clean install of
openai==3.0.0plus this packagehttpxcan be absent entirely andthe module-scope
from httpx import URLmade the package unimportable rather than merely degraded.This looks like a spot missed by the recent httpx2 migration (#4730, #4948) rather than a separate
problem.
Behaviour is preserved on every path that works today:
base_urlhttpx.URL.host/.porturlparse:443(None, None)httpx2.URL(None, None).host/.port(None, None).host/.portType of change
How Has This Been Tested?
New
tests/test_server_address.pycoveringhttpx.URL,httpx2.URL, a plain string, an explicit:443(asserting the port is still omitted), a missingbase_url, and that the package imports withhttpxuninstalled. The last one is skipped onopenai2.x, which depends onhttpxitself, so thesituation cannot arise there.
Verified in two environments:
openai==2.26.0,httpx==0.27.2): full package suite 193 passed, 28skipped, no change in behaviour;
openai==3.0.0withhttpxgenuinely absent: the package imports and the new tests pass. Revertingthe fix in that environment makes the test module fail at import with
ModuleNotFoundError: No module named 'httpx', which is the regression this guards.Worth noting:
tests/requirements.latest.txtpinsopenai==2.26.0, so nothing in CI currentlyexercises
openai3.x. Happy to addhttpx2there, or to bump the pin in a separate PR, if you wouldlike the new cases to run in CI rather than skip.
Does This PR Require a Core Repo Change?
Checklist:
.changelog/4954.fixed)