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>
115 lines
3.6 KiB
Markdown
115 lines
3.6 KiB
Markdown
---
|
|
name: python-development
|
|
description: >
|
|
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:
|
|
|
|
```python
|
|
# Copyright (c) Microsoft. All rights reserved.
|
|
```
|
|
|
|
## Type Annotations
|
|
|
|
- Always specify return types and parameter types
|
|
- Use `Type | None` instead of `Optional[Type]`
|
|
- Use `from __future__ import annotations` to enable postponed evaluation
|
|
- Use suffix `T` for TypeVar names: `ChatResponseT = TypeVar("ChatResponseT", bound=ChatResponse)`
|
|
- Use `Mapping` instead of `MutableMapping` for read-only input parameters
|
|
- Prefer `# type: ignore[...]` over unnecessary casts, or `isinstance` checks, 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:
|
|
|
|
```python
|
|
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_handler` instead of `next`)
|
|
- Avoid `**kwargs` unless needed for subclass extensibility; prefer named parameters
|
|
|
|
## Docstrings
|
|
|
|
Use Google-style docstrings for all public APIs:
|
|
|
|
```python
|
|
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 Args` when applicable
|
|
- Only document standard Python exceptions when the condition is non-obvious
|
|
|
|
## Import Structure
|
|
|
|
```python
|
|
# 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__`.
|
|
|
|
```python
|
|
__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/case` on `.type` attribute over `isinstance()` 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
|