Skip to content

Python: Fix FoundryAgent telemetry gaps for name and model (Fixes #5088) #5094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Charanvardhan wants to merge 2 commits into from
Closed

Python: Fix FoundryAgent telemetry gaps for name and model (Fixes #5088) #5094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d91105d
Python: Fix FoundryAgent telemetry gaps for name and model (Fixes #5088)
Charanvardhan Apr 4, 2026
a94ca20
Address Copilot PR review comments
Charanvardhan Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion python/packages/foundry/agent_framework_foundry/_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ def _get_agent_reference(self) -> dict[str, str]:
ref["version"] = self.agent_version
return ref

@property
def model(self) -> str:
"""Get the lazily fetched model name, or 'unknown' if not yet fetched."""
return getattr(self, "_fetched_model", "unknown")

Comment thread
Charanvardhan marked this conversation as resolved.
@model.setter
def model(self, value: str) -> None:
pass

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

model is defined as a property, but the setter is a no-op. Because RawFoundryAgentChatClient subclasses RawOpenAIChatClient, RawOpenAIChatClient.__init__ assigns self.model = ... (openai/_chat_client.py:446); with the current no-op setter, that assignment is silently discarded, and any later client.model = ... will also do nothing. Implement the setter to persist the value (e.g., set the backing field used by the getter) or remove the property and use a normal attribute/backing field so assignments behave correctly.

Suggested change
pass
self._fetched_model = value

Copilot uses AI. Check for mistakes.

@override
def as_agent(
self,
Expand Down Expand Up @@ -269,6 +278,29 @@ async def _prepare_options(
**kwargs: Any,
) -> dict[str, Any]:
"""Prepare options for the Responses API, injecting agent reference and validating tools."""
# Lazily fetch the model name if not already cached
if not hasattr(self, "_fetched_model"):
try:
agent = await self.project_client.agents.get_agent(self.agent_name)
self._fetched_model = getattr(agent, "model", "unknown")
except Exception:
self._fetched_model = "unknown"

Comment on lines +281 to +288

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

The lazy model fetch uses if not hasattr(self, "_fetched_model") and then awaits project_client.agents.get_agent(...). If two calls enter _prepare_options concurrently before _fetched_model is set, both will perform the network call. Consider guarding the lazy initialization with an asyncio.Lock or an in-flight Task/Future so only one fetch occurs and others await it.

Copilot uses AI. Check for mistakes.
# Try to update the current OpenTelemetry span directly
if hasattr(self, "_fetched_model") and self._fetched_model != "unknown":
try:
from opentelemetry import trace
from agent_framework.observability import OtelAttr

current_span = trace.get_current_span()
if current_span and current_span.is_recording():
current_span.set_attribute(OtelAttr.REQUEST_MODEL, self._fetched_model)
span_name_parts = current_span.name.split(" ", 1)
if len(span_name_parts) > 0:
current_span.update_name(f"{span_name_parts[0]} {self._fetched_model}")
except Exception:
Comment on lines +289 to +301

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

Updating the OpenTelemetry span via trace.get_current_span() won’t work for streaming requests: ChatTelemetryLayer intentionally creates streaming spans without context attachment (core/agent_framework/observability.py:1293-1299), so there may be no “current span” to update. As a result, gen_ai.request.model can still remain unknown for stream=True. Consider a mechanism that ensures the span used for the request is the one being updated (e.g., providing the model before span creation, or passing/propagating the span explicitly).

Copilot uses AI. Check for mistakes.
pass
Comment on lines +283 to +302

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

The except Exception blocks here swallow all failures (including auth/config/service errors) and then suppress them again when updating OTEL, with no logging. This can make real regressions very hard to diagnose and permanently cache "unknown" after a transient error. Prefer catching expected exception types (e.g., ImportError separately, and the specific Azure SDK error types for get_agent) and logging at least a debug/warning message once when resolution fails.

Copilot uses AI. Check for mistakes.

# Validate tools — only FunctionTool allowed
tools = options.get("tools", [])
if tools:
Expand Down Expand Up @@ -543,11 +575,13 @@ def __init__(

client = actual_client_type(**client_kwargs)

resolved_name = name if name is not None else getattr(client, "agent_name", None)

super().__init__(
client=client, # type: ignore[arg-type]
instructions=instructions,
id=id,
name=name,
name=resolved_name,
description=description,
tools=tools, # type: ignore[arg-type]
default_options=cast(FoundryAgentOptionsT | None, default_options),
Expand Down
35 changes: 35 additions & 0 deletions python/packages/foundry/tests/foundry/test_foundry_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,38 @@ async def test_foundry_agent_custom_client_run() -> None:
assert isinstance(response, AgentResponse)
assert response.text is not None
assert "response test" in response.text.lower()


async def test_foundry_agent_telemetry_defaults() -> None:
"""Test that agent name acts as a fallback and _prepare_options lazily gets model."""
mock_project = MagicMock()
mock_openai = MagicMock()
mock_project.get_openai_client.return_value = mock_openai

# Mock agents getter
mock_agent_instance = MagicMock()
mock_agent_instance.model = "gpt-telemetry-test"
mock_project.agents.get_agent = AsyncMock(return_value=mock_agent_instance)

agent = FoundryAgent(
project_client=mock_project,
agent_name="my-telemetry-agent",
name=None # Explicitly None to test fallback

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

This multiline FoundryAgent(...) call is missing a trailing comma after the last argument. In this repo most multiline call sites include trailing commas (and formatters like Black will typically add them), so adding it will avoid churn / formatting diffs.

Suggested change
name=None # Explicitly None to test fallback
name=None, # Explicitly None to test fallback

Copilot uses AI. Check for mistakes.
)

assert agent.name == "my-telemetry-agent"
assert getattr(agent.client, "model", "unknown") == "unknown"

# Call prepare_options to trigger lazy load
with patch(
"agent_framework_openai._chat_client.RawOpenAIChatClient._prepare_options",
new_callable=AsyncMock,
return_value={},
):
await agent.client._prepare_options(
messages=[Message(role="user", contents="hi")],
options={},
)

assert agent.client.model == "gpt-telemetry-test"
mock_project.agents.get_agent.assert_called_once_with("my-telemetry-agent")
Loading