Skip to content

Python: Add service_url to OllamaChatClient - #9222

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
BichengWang:ollama-chat-client-service-url
Oct 9, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
BichengWang:ollama-chat-client-service-url

Conversation

@BichengWang

@BichengWang Bicheng (Kenneth) (BichengWang) commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

OllamaChatClient does not override BaseChatClient.service_url(), so chat spans report server.address as Unknown. OllamaEmbeddingClient and the Anthropic, Bedrock, Gemini, Mistral and OpenAI chat-completion clients already implement it.

Description & Review Guide

  • What are the major changes? OllamaChatClient.service_url() returns self.host, which is already populated in __init__. One unit test added.
  • What is the impact of these changes? Ollama chat spans now carry the real server address. Nothing else changes. The value is the full URL (with httpx's trailing slash), the same as the embedding client and the other providers emit.
  • What do you want reviewers to focus on? Nothing specific.

Related Issue

Fixes #9221

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Report the Ollama host as server.address in chat telemetry instead of
the base class default of Unknown, matching OllamaEmbeddingClient and
the other provider chat clients.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation matches existing provider behavior and includes appropriate unit coverage.

0 open findings

What changed in this PR

Adds Ollama chat service URL reporting for telemetry.

Changes:

  • Returns the resolved Ollama host from service_url().
  • Adds unit coverage for a custom host.
File Description
python/​packages/​ollama/​agent_framework_ollama/​_chat_client.py Exposes the resolved host for telemetry.
python/​packages/​ollama/​tests/​test_ollama_chat_client.py Tests service URL reporting.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@BichengWang

Copy link
Copy Markdown
Contributor Author

Hello members, is there any one can help additional review? Thank you!

Merged via the queue into microsoft:main with commit b2ee384 Oct 9, 2026
50 of 51 checks passed
@BichengWang
Bicheng (Kenneth) (BichengWang) deleted the ollama-chat-client-service-url branch October 9, 2026 17:57

This branch was previously deployed

1 inactive deployment
github-app-auth — a8a20c9a Deployed Oct 8, 2026 by BichengWang via team_check #6406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: OllamaChatClient does not implement service_url(), so server.address is reported as Unknown

3 participants