[agentserver] Invoke and Core package refactor#46127
Conversation
- Added docstring for TracingHelper.__init__ connection_string param - Added enrichment processor dupe guard (_enrichment_configured flag) - Fixed InMemorySpanExporter import path in test_tracing.py - Fixed @app → @server variable name mismatch in test_tracing.py - Updated invocations CHANGELOG with 2.0.0b1 + kept 1.0.0b1 history - Fixed duplicate InvocationAgentServerHost imports in README - Fixed README titles to match Verify Readmes pattern - Fixed tracing tests to use TestClient and set APPLICATIONINSIGHTS env var - Fixed test pollution: OTel provider reused across test modules - Removed obsolete baggage test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Core: added historical 1.0.0b1 entry below 2.0.0b1, removed stale leaf_customer_span_id from features. Invocations: reverted to single 1.0.0b1 entry (new package, no prior releases). Updated feature list to reflect InvocationAgentServerHost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaced the TracingHelper class with: - configure_tracing() — standalone function for exporter setup, overridable via AgentServerHost(configure_tracing=my_func) or disabled with configure_tracing=None - request_span() — module-level context manager for span creation - end_span/record_error/trace_stream — module-level lifecycle helpers - AgentServerHost.request_span() — thin method that delegates with pre-populated host identity (agent_id, project_id) Protocol SDKs now use self.request_span() instead of self._tracing.request_span() with None checks. All functions are no-ops when opentelemetry-api is not installed. Removed TracingHelper from core __init__.py exports. Agent identity (name/version/project_id) moved to AgentServerHost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed TracingHelper import, contextlib.nullcontext pattern, and None checks. Now uses self.request_span() and _tracing.record_error(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
get_logger() was a one-liner wrapping logging.getLogger('azure.ai.agentserver').
Replaced all usages with direct logging.getLogger() calls and deleted _logger.py.
Removed get_logger from core __init__.py exports.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the AgentServer tracing and logging surface, moving from class-based helpers to function-based tracing setup and host-provided span helpers, and updates the invocations protocol and tests accordingly.
Changes:
- Refactor core tracing from
TracingHelperto function-based APIs (configure_tracing,request_span,end_span,record_error,trace_stream) and addAgentServerHost.request_span(). - Update invocations host and tests to use the new tracing APIs and switch tests from
httpxASGI transport tostarlette.testclient.TestClient. - Refresh READMEs/CHANGELOGs to align with the invocations host model and updated tracing behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/agentserver/azure-ai-agentserver-invocations/tests/test_tracing.py | Updates tracing tests to new tracing implementation and TestClient usage. |
| sdk/agentserver/azure-ai-agentserver-invocations/tests/test_span_parenting.py | Adjusts span parenting tests for the updated tracing/context behavior. |
| sdk/agentserver/azure-ai-agentserver-invocations/README.md | Updates package title and removes duplicated imports in snippets. |
| sdk/agentserver/azure-ai-agentserver-invocations/CHANGELOG.md | Updates feature list to reflect InvocationAgentServerHost and mixin model. |
| sdk/agentserver/azure-ai-agentserver-invocations/azure/ai/agentserver/invocations/_invocation.py | Moves invocations tracing to host span helper + core tracing functions. |
| sdk/agentserver/azure-ai-agentserver-core/tests/test_tracing.py | Updates tests to validate configure_tracing injection behavior. |
| sdk/agentserver/azure-ai-agentserver-core/samples/selfhosted_invocation/selfhosted_invocation.py | Updates sample to use AgentServerHost.request_span() and new tracing helpers. |
| sdk/agentserver/azure-ai-agentserver-core/README.md | Updates package title. |
| sdk/agentserver/azure-ai-agentserver-core/CHANGELOG.md | Updates breaking-change notes for tracing/logging behavior. |
| sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_tracing.py | Implements the function-based tracing API and exporter setup. |
| sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_logger.py | Removes the old get_logger() facade module. |
| sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py | Adds configure_tracing injection + request_span() convenience method. |
| sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/init.py | Updates exported public API surface. |
sdk/agentserver/azure-ai-agentserver-invocations/tests/test_tracing.py
Outdated
Show resolved
Hide resolved
| @pytest.mark.asyncio | ||
| async def test_tracing_disabled_by_default(): | ||
| def test_tracing_disabled_by_default(): | ||
| """No spans are created when tracing is not enabled.""" |
There was a problem hiding this comment.
This docstring no longer matches the assertion below (the test now expects at least one invoke span even when tracing isn't configured). Please update the docstring to reflect the new behavior being validated.
| """No spans are created when tracing is not enabled.""" | |
| """Invoke spans are still created by the global tracer when tracing is not explicitly configured.""" |
sdk/agentserver/azure-ai-agentserver-invocations/tests/test_tracing.py
Outdated
Show resolved
Hide resolved
| with patch.dict(os.environ, { | ||
| "FOUNDRY_AGENT_NAME": "my-agent", | ||
| "FOUNDRY_AGENT_VERSION": "2.0", | ||
| "FOUNDRY_AGENT_NAME": "my-agent", | ||
| "FOUNDRY_AGENT_VERSION": "2.0", | ||
| }): |
There was a problem hiding this comment.
The dict literal passed to patch.dict() is not indented inside the braces. While syntactically valid, it is not Black-formatted and is likely to fail formatting checks; please reformat the with-block arguments.
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_tracing.py
Outdated
Show resolved
Hide resolved
| Constants, | ||
| create_error_response, | ||
| ) | ||
| from azure.ai.agentserver.core import _tracing |
There was a problem hiding this comment.
azure.ai.agentserver.core._tracing is an internal (underscore) module. Importing it from the invocations package creates a private cross-package dependency that’s hard to evolve. Prefer going through AgentServerHost methods, or expose these helpers via a public module/API in core so invocations doesn’t rely on internals.
| from azure.ai.agentserver.core import _tracing |
| @@ -252,10 +253,7 @@ def _wrap_streaming_response( | |||
| :return: The same response object, with its body_iterator replaced. | |||
| :rtype: ~starlette.responses.StreamingResponse | |||
| """ | |||
There was a problem hiding this comment.
_wrap_streaming_response wraps the body iterator even when otel_span is None (e.g., when opentelemetry isn’t installed). This adds avoidable per-chunk overhead for streaming responses. Consider short-circuiting and returning the original response when otel_span is None.
| """ | |
| """ | |
| if otel_span is None: | |
| return response |
| from azure.ai.agentserver.core import AgentServerHost | ||
| from azure.ai.agentserver.core import _tracing | ||
|
|
There was a problem hiding this comment.
This sample imports the internal azure.ai.agentserver.core._tracing module. Because samples are copied verbatim by customers, this encourages depending on non-public APIs. Consider either (a) using only AgentServerHost.request_span() + span APIs, or (b) providing a public tracing module/function exports in core for record_error / end_span.
| from ._base import AgentServerHost | ||
| from ._constants import Constants | ||
| from ._errors import create_error_response | ||
| from ._logger import get_logger | ||
| from ._tracing import TracingHelper | ||
| from ._version import VERSION | ||
|
|
||
| __all__ = [ | ||
| "AgentServerHost", | ||
| "Constants", | ||
| "create_error_response", | ||
| "get_logger", | ||
| "TracingHelper", | ||
| ] |
There was a problem hiding this comment.
get_logger has been removed from the core package, but other agentserver packages in this repo still import azure.ai.agentserver.core.logger.get_logger. Without updating those packages or providing a compatibility shim (e.g., reintroducing core/logger.py that returns logging.getLogger("azure.ai.agentserver")), this change will break imports at runtime.
| - Replaced `AgentLogger.get()` static method with module-level `get_logger()` function. | ||
| - Removed `AGENT_LOG_LEVEL` and `AGENT_GRACEFUL_SHUTDOWN_TIMEOUT` environment variable support from `Constants`. | ||
| - Removed `leaf_customer_span_id` baggage mechanism and W3C Baggage propagation. | ||
| - Simplified `TracingHelper` API — removed `set_current_span`, `detach_context`, `set_baggage`, `detach_baggage`. | ||
| - Renamed health endpoint from `/healthy` to `/readiness`. |
There was a problem hiding this comment.
These breaking-change bullets still describe a TracingHelper API, but the implementation is now function-based and TracingHelper is no longer part of the public surface. Please update the changelog entries to match the new tracing API so release notes are accurate.
Moved opentelemetry-api, opentelemetry-sdk, opentelemetry-exporter-otlp, and azure-monitor-opentelemetry-exporter from optional [tracing] extras to primary dependencies. Removed _HAS_OTEL flag, try/except import guard, and all conditional checks — OTel is always available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…acing.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…acing.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/core/_tracing.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…date CHANGELOG - Exported end_span, record_error, trace_stream from core __init__.py (no more importing internal _tracing module from other packages) - Updated invocations to use public imports from core - Updated selfhosted sample to use public record_error import - Added None guard in _wrap_streaming_response for otel_span - Fixed test docstring mismatch (tracing_disabled_by_default) - Updated CHANGELOG to reflect TracingHelper → functions change - Fixed get_logger import in githubcopilot adapter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dk-for-python into agentserver/invoke
… isolation headers, HTTP/2 - x-platform-server header now includes version and python runtime - Instrumentation scope: Azure.AI.AgentServer (core), .Invocations (invocations) - record_error() now sets error.type attribute per OTel semantic conventions - baggage header included in W3C trace context extraction - x-request-id propagated into span attributes - Platform isolation headers (x-agent-user-isolation-key, x-agent-chat-isolation-key) exposed via request.state - HTTP/2 disabled in Hypercorn config (spec requires HTTP/1.1 only) - Fixed get_logger import in githubcopilot adapter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…GTERM forwarding - Re-added W3C baggage propagation for invocation_id/session_id - Added SSE_KEEPALIVE_INTERVAL env var and resolve_sse_keepalive_interval() - Added sse_keepalive_stream() as AgentServerHost static method (not in tracing) - Added _InvocationLogFilter for structured log scope with InvocationId/SessionId - Added SIGTERM handler in run() that logs and re-raises for Hypercorn - Separated trace_stream (tracing concern) from sse_keepalive_stream (transport) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines