mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
72a6157c6a
* 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>
3.6 KiB
3.6 KiB
name, description
| name | description |
|---|---|
| python-development | Coding standards, conventions, and patterns for developing Python code in the Agent Framework repository. Use this when writing or modifying Python source files in the python/ directory. |
Python Development Standards
File Header
Every .py file must start with:
# Copyright (c) Microsoft. All rights reserved.
Type Annotations
- Always specify return types and parameter types
- Use
Type | Noneinstead ofOptional[Type] - Use
from __future__ import annotationsto enable postponed evaluation - Use suffix
Tfor TypeVar names:ChatResponseT = TypeVar("ChatResponseT", bound=ChatResponse) - Use
Mappinginstead ofMutableMappingfor read-only input parameters - Prefer
# type: ignore[...]over unnecessary casts, orisinstancechecks, when these are internally called and executed methods But make sure the ignore is specific for both mypy and pyright so that we don't miss other mistakes
Function Parameters
- Positional parameters: up to 3 fully expected parameters
- Use keyword-only arguments (after
*) for optional parameters - Provide string-based overrides to avoid requiring extra imports:
def create_agent(name: str, tool_mode: Literal['auto', 'required', 'none'] | ChatToolMode) -> Agent:
if isinstance(tool_mode, str):
tool_mode = ChatToolMode(tool_mode)
- Avoid shadowing built-ins (use
next_handlerinstead ofnext) - Avoid
**kwargsunless needed for subclass extensibility; prefer named parameters
Docstrings
Use Google-style docstrings for all public APIs:
def equal(arg1: str, arg2: str) -> bool:
"""Compares two strings and returns True if they are the same.
Args:
arg1: The first string to compare.
arg2: The second string to compare.
Returns:
True if the strings are the same, False otherwise.
Raises:
ValueError: If one of the strings is empty.
"""
- Always document Agent Framework specific exceptions
- Explicitly use
Keyword Argswhen applicable - Only document standard Python exceptions when the condition is non-obvious
Import Structure
# Core
from agent_framework import Agent, Message, tool
# Components
from agent_framework.observability import enable_sensitive_telemetry
# Connectors (lazy-loaded)
from agent_framework.openai import OpenAIChatClient
from agent_framework.foundry import FoundryChatClient
Public API and Exports
In __init__.py files that define package-level public APIs, use direct re-export imports plus an explicit
__all__. Avoid identity aliases like from ._agents import Agent as Agent, and avoid
from module import *.
Do not define __all__ in internal non-__init__.py modules. Exception: modules intentionally exposed as a
public import surface (for example, agent_framework.observability) should define __all__.
__all__ = ["Agent", "Message", "ChatResponse"]
from ._agents import Agent
from ._types import Message, ChatResponse
Performance Guidelines
- Cache expensive computations (e.g., JSON schema generation)
- Prefer
match/caseon.typeattribute overisinstance()in hot paths - Avoid redundant serialization — compute once, reuse
Style
- Line length: 120 characters
- Format only files you changed, not the entire codebase
- Prefer attributes over inheritance when parameters are mostly the same
- Async by default — assume everything is asynchronous
Naming Conventions for Connectors
_prepare_<object>_for_<purpose>for methods that prepare data for external services_parse_<object>_from_<source>for methods that process data from external services