[BREAKING] Python: Enable instrumentation by default (#5865) · alliscode/agent-framework@72a6157 · GitHub
Skip to content

Commit 72a6157

Browse files
TaoChenOSUeavanvalkenburgCopilot
authored
[BREAKING] Python: Enable instrumentation by default (microsoft#5865)
* Enable instrumentation by default * Update samples * Optimization when span is not recording * Address Copilot comments * Revert uv.lock * Add warning * Formatting * Fix mypy * Add disable_instrumentation() with sticky user-intent semantics Add a public disable_instrumentation() entry point so users can explicitly opt out of Agent Framework telemetry, with a sticky-disable flag that makes the user's intent "leading" — no framework code path (foundry's configure_azure_monitor, configure_otel_providers, enable_instrumentation, enable_sensitive_telemetry, or direct OBSERVABILITY_SETTINGS.enable_* writes) can re-enable instrumentation until the user explicitly clears the disable with enable_instrumentation(force=True) / enable_sensitive_telemetry(force=True). Also addresses the two remaining unresolved review threads on the PR: 1. test_observability_settings_defaults_instrumentation_true pins the new "ENABLE_INSTRUMENTATION defaults to True when env unset" behavior. 2. test_enable_instrumentation_reads_env_sensitive_data restores coverage for the post-import load_dotenv() fallback path. Implementation: - ObservabilitySettings.enable_instrumentation / enable_sensitive_data become properties backed by _enable_*. While _user_disabled is True, the getters return False and the setters drop True writes (defense in depth so third- party writes can't subvert the disable). - Public is_user_disabled read-only property lets integrations (e.g. foundry's configure_azure_monitor) cheaply check the disable state without poking at privates. - enable_instrumentation() and enable_sensitive_telemetry() short-circuit with an info log when disabled; gain a force=True kwarg that clears the disable. - configure_otel_providers() still creates providers / exporters / views so a later force-enable can use them, but logs an info message when called while disabled. - Foundry's FoundryChatClient.configure_azure_monitor and FoundryAgent.configure_azure_monitor early-return when the user has disabled, so Azure Monitor's global providers aren't installed unnecessarily. Tests: 11 new tests covering default-on, env re-read at call time, sticky behavior against each re-enable surface (enable_instrumentation, enable_sensitive_telemetry, configure_otel_providers, direct attribute writes), force=True override, re-arming the disable, and the __all__ export. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: document disable_instrumentation() and force=True paths Add a "Disabling instrumentation" section to the observability sample README that walks through: - The distinction between the ENABLE_INSTRUMENTATION env var (initial, non-sticky) and disable_instrumentation() (process-wide, sticky). - Why the sticky semantics matter: framework integrations like FoundryChatClient.configure_azure_monitor() can call enable_instrumentation() as part of their setup, and the user's opt-out needs to win. - All five surfaces guarded by the sticky disable (property reads, public enable functions, configure_otel_providers, direct attribute writes, is_user_disabled-aware integrations). - The force=True escape hatch on both enable_instrumentation() and enable_sensitive_telemetry(). - How third-party integrations should consult OBSERVABILITY_SETTINGS.is_user_disabled. - The limits of the disable (does not tear down existing providers / in-flight spans / third-party instrumentation, does not persist across processes). Cross-links the new section from the ENABLE_INSTRUMENTATION row in the env vars table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: soften disable_instrumentation() overclaim about telemetry guarantees Replace 'no telemetry will be emitted no matter what' (which is too strong, since callers can still pass force=True or mutate private attributes) with language framing the disable as a user-intent contract that library and framework code is expected to honor: the framework actively short-circuits the public enable paths, force=True and private-attribute writes are acknowledged as out-of-contract escape hatches that integrations should not use on the user's behalf. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct observability Dependencies section - opentelemetry-sdk is no longer a hard dependency; it is lazily imported by create_resource(), create_metric_views(), and configure_otel_providers() with a clear ImportError when missing. Day-to-day instrumentation works with opentelemetry-api alone provided some other component configures the global OpenTelemetry providers (Azure Monitor, an APM agent, application bootstrap, etc.). - opentelemetry-semantic-conventions-ai is no longer used anywhere in the source; remove it from the listed dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: replace stale observability migration guide with current PR's only relevant migration The old guide documented the move away from setup_observability(otlp_endpoint=...) which was an earlier-release API change unrelated to this PR and stale enough that it's more confusing than helpful at this point. Replace it with a short note on the single migration this PR introduces: callers of enable_instrumentation(enable_sensitive_data=True) should switch to enable_sensitive_telemetry(). Cross-link to the Disabling instrumentation section for the rare 'force on without enabling sensitive data' use case where enable_instrumentation() still applies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0ba552b commit 72a6157

20 files changed

Lines changed: 881 additions & 318 deletions

File tree

‎python/.env.example‎

Lines changed: 1 addition & 2 deletions

‎python/.github/skills/python-development/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def equal(arg1: str, arg2: str) -> bool:
7272
from agent_framework import Agent, Message, tool
7373

7474
# Components
75-
from agent_framework.observability import enable_instrumentation
75+
from agent_framework.observability import enable_sensitive_telemetry
7676

7777
# Connectors (lazy-loaded)
7878
from agent_framework.openai import OpenAIChatClient

‎python/CODING_STANDARD.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ The package follows a flat import structure:
186186

187187
- **Components**: Import from `agent_framework.<component>`
188188
```python
189-
from agent_framework.observability import enable_instrumentation, configure_otel_providers
189+
from agent_framework.observability import enable_sensitive_telemetry, configure_otel_providers
190190
```
191191

192192
- **Connectors**: Import from `agent_framework.<vendor/platform>`

‎python/packages/chatkit/agent_framework_chatkit/_converter.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from __future__ import annotations
66

77
import logging
8-
import sys
98
from collections.abc import Awaitable, Callable, Sequence
109

1110
from agent_framework import (
@@ -31,11 +30,6 @@
3130
WorkflowItem,
3231
)
3332

34-
if sys.version_info >= (3, 11):
35-
from typing import assert_never # type:ignore # pragma: no cover
36-
else:
37-
from typing_extensions import assert_never # type:ignore # pragma: no cover
38-
3933
logger = logging.getLogger(__name__)
4034

4135

@@ -532,7 +526,10 @@ async def _thread_item_to_input_item(
532526
# TODO(evmattso): Implement structured input handling in a future PR
533527
return []
534528
case _:
535-
assert_never(item)
529+
# Unknown ThreadItem variant (e.g. types added in newer chatkit versions).
530+
# Skip rather than fail so we remain forward-compatible with chatkit upgrades.
531+
logger.debug("Skipping unsupported ThreadItem of type %s", type(item).__name__)
532+
return []
536533

537534
async def to_agent_input(
538535
self,

‎python/packages/core/agent_framework/observability.py‎

Lines changed: 173 additions & 23 deletions
Large diffs are not rendered by default.

‎python/packages/core/tests/core/test_observability.py‎

Lines changed: 447 additions & 66 deletions
Large diffs are not rendered by default.

‎python/packages/foundry/agent_framework_foundry/_agent.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,8 +793,22 @@ async def configure_azure_monitor(
793793
Raises:
794794
ImportError: If azure-monitor-opentelemetry-exporter is not installed.
795795
"""
796+
from agent_framework.observability import (
797+
OBSERVABILITY_SETTINGS,
798+
create_metric_views,
799+
create_resource,
800+
enable_instrumentation,
801+
)
796802
from azure.core.exceptions import ResourceNotFoundError
797803

804+
if OBSERVABILITY_SETTINGS.is_user_disabled:
805+
logger.info(
806+
"FoundryAgent.configure_azure_monitor(): Skipping setup because instrumentation was "
807+
"explicitly disabled via disable_instrumentation(). Call enable_instrumentation(force=True) "
808+
"to re-enable, then re-invoke configure_azure_monitor()."
809+
)
810+
return
811+
798812
client = self.client
799813
if not isinstance(client, RawFoundryAgentChatClient):
800814
raise TypeError("configure_azure_monitor requires a RawFoundryAgentChatClient-based client.")
@@ -817,8 +831,6 @@ async def configure_azure_monitor(
817831
"Install it with: pip install azure-monitor-opentelemetry"
818832
) from exc
819833

820-
from agent_framework.observability import create_metric_views, create_resource, enable_instrumentation
821-
822834
if "resource" not in kwargs:
823835
kwargs["resource"] = create_resource()
824836

‎python/packages/foundry/agent_framework_foundry/_chat_client.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,22 @@ async def configure_azure_monitor(
271271
Raises:
272272
ImportError: If azure-monitor-opentelemetry-exporter is not installed.
273273
"""
274+
from agent_framework.observability import (
275+
OBSERVABILITY_SETTINGS,
276+
create_metric_views,
277+
create_resource,
278+
enable_instrumentation,
279+
)
274280
from azure.core.exceptions import ResourceNotFoundError
275281

282+
if OBSERVABILITY_SETTINGS.is_user_disabled:
283+
logger.info(
284+
"FoundryChatClient.configure_azure_monitor(): Skipping setup because instrumentation was "
285+
"explicitly disabled via disable_instrumentation(). Call enable_instrumentation(force=True) "
286+
"to re-enable, then re-invoke configure_azure_monitor()."
287+
)
288+
return
289+
276290
try:
277291
conn_string = await self.project_client.telemetry.get_application_insights_connection_string()
278292
except ResourceNotFoundError:
@@ -291,8 +305,6 @@ async def configure_azure_monitor(
291305
"Install it with: pip install azure-monitor-opentelemetry"
292306
) from exc
293307

294-
from agent_framework.observability import create_metric_views, create_resource, enable_instrumentation
295-
296308
if "resource" not in kwargs:
297309
kwargs["resource"] = create_resource()
298310

‎python/samples/02-agents/observability/.env.example‎

Lines changed: 3 additions & 3 deletions

0 commit comments

Comments
 (0)