[BREAKING] Python: Enable instrumentation by default

[TaoChenOSU]

Motivation and Context

Closes #5749

Description

Changes required to make the code base consistent throughout.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the Python Agent Framework and samples to reflect that instrumentation is now enabled by default, and shifts programmatic opt-in to a new enable_sensitive_telemetry() API for sensitive payload capture.

Changes:

• Make ENABLE_INSTRUMENTATION default to enabled (opt-out via false) and update docs/samples accordingly.
• Replace most uses of enable_instrumentation() with enable_sensitive_telemetry() for sensitive-data opt-in.
• Update DevUI and Foundry integration points to reflect the new defaults/entry points.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
python/samples/README.md	Updates env var table to reference observability settings instead of enable_instrumentation()
python/samples/04-hosting/foundry-hosted-agents/responses/07_observability/agent.yaml	Removes ENABLE_INSTRUMENTATION from sample agent config; keeps sensitive-data opt-in
python/samples/04-hosting/foundry-hosted-agents/responses/07_observability/agent.manifest.yaml	Removes ENABLE_INSTRUMENTATION from manifest sample
python/samples/04-hosting/foundry-hosted-agents/responses/07_observability/README.md	Documents instrumentation default-on and opt-out
python/samples/04-hosting/foundry-hosted-agents/responses/07_observability/.env.example	Removes ENABLE_INSTRUMENTATION example line
python/samples/03-workflows/observability/executor_io_observation.py	Wording update to reflect default instrumentation
python/samples/02-agents/observability/advanced_manual_setup_console_output.py	Switches to enable_sensitive_telemetry() for sensitive data capture
python/samples/02-agents/observability/README.md	Updates guidance/code samples to default-on instrumentation + sensitive opt-in
python/samples/02-agents/observability/.env.example	Documents opt-out via ENABLE_INSTRUMENTATION=false
python/packages/lab/lightning/agent_framework_lab_lightning/init.py	Removes explicit enable_instrumentation() call from tracer init
python/packages/foundry/tests/foundry/test_foundry_chat_client.py	Updates mocks to enable_sensitive_telemetry()
python/packages/foundry/tests/foundry/test_foundry_agent.py	Updates mocks to enable_sensitive_telemetry()
python/packages/foundry/agent_framework_foundry/_chat_client.py	Calls enable_sensitive_telemetry() only when sensitive data is requested
python/packages/foundry/agent_framework_foundry/_agent.py	Calls enable_sensitive_telemetry() only when sensitive data is requested
python/packages/devui/frontend/src/components/layout/deployment-modal.tsx	Updates docker-compose snippet to reflect default-on instrumentation
python/packages/devui/agent_framework_devui/_server.py	Treats instrumentation as enabled unless explicitly set to false
python/packages/devui/agent_framework_devui/_executor.py	Updates comment to reflect new defaults/entry point
python/packages/devui/agent_framework_devui/init.py	Removes instrumentation_enabled parameter and related logic
python/packages/core/tests/core/test_observability.py	Updates tests to cover the new enable_sensitive_telemetry() behavior
python/packages/core/agent_framework/observability.py	Defaults instrumentation to enabled; replaces enable_instrumentation() with enable_sensitive_telemetry()
python/CODING_STANDARD.md	Updates import example to enable_sensitive_telemetry
python/.github/skills/python-development/SKILL.md	Updates skills doc sample import to enable_sensitive_telemetry
python/.env.example	Updates env example to reflect default-on instrumentation

[moonbox3]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/a2a/agent_framework_a2a
_agent.py	277	18	93%	162, 349, 354, 356, 400–401, 554, 570, 578, 598, 619, 647, 663, 673, 684, 691–692, 733
packages/chatkit/agent_framework_chatkit
_converter.py	135	49	63%	112, 117, 165, 167, 335, 388, 390, 409–411, 413, 431, 433, 435, 438, 450, 460, 478, 498–522, 524–525, 527–528, 531–532
packages/core/agent_framework
_skills.py	874	26	97%	289, 531, 541, 977, 992, 994–995, 1549, 1578, 2544–2545, 2680, 2685, 2688, 2693, 2720, 2725, 2779, 2788, 2793, 2796, 2801, 2825, 2830, 3072–3073
observability.py	775	61	92%	380, 382–383, 386, 389, 392–393, 398–399, 405–406, 412–413, 420, 422–424, 427–429, 434–435, 441–442, 448–449, 456, 613–614, 750, 754–756, 758, 762–763, 767, 805, 807, 818–820, 822–824, 828, 836, 960–961, 1139, 1382–1383, 1486, 1610, 1651–1652, 1795, 1931, 2128, 2346, 2348
packages/openai/agent_framework_openai
_chat_client.py	1045	139	86%	276, 289, 614–617, 621–622, 628–629, 664–670, 677–679, 700, 708, 731, 849, 948, 1007, 1009, 1011, 1013, 1079, 1093, 1173, 1183, 1188, 1231, 1342–1343, 1358, 1567, 1572, 1576–1578, 1582–1583, 1666, 1676, 1703, 1709, 1719, 1725, 1730, 1736, 1741–1742, 1761, 1764–1767, 1781, 1783, 1791–1792, 1804, 1846, 1936, 1958–1959, 1974–1975, 1993–1994, 2037, 2203, 2241–2242, 2258, 2260, 2340–2348, 2378, 2488, 2523, 2538, 2558–2568, 2581, 2592–2596, 2610, 2624–2635, 2644, 2676–2679, 2689–2690, 2701–2703, 2717–2719, 2729–2730, 2736, 2751
TOTAL	34124	3894	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
6804	30 💤	0 ❌	0 🔥	1m 45s ⏱️

[github-actions]

Automated Code Review

Reviewers: 4 | Confidence: 90%

✗ Correctness

The PR correctly implements the 'enable instrumentation by default' change in ObservabilitySettings and renames enable_instrumentation to enable_sensitive_telemetry. However, beyond the already-flaged issues in the review thread, the configure_azure_monitor() methods in both _agent.py and _chat_client.py have a security-relevant behavioral regression: the old code always actively set enable_sensitive_data to False by default (overriding the env var), whereas the new code is a no-op when enable_sensitive_data=False, meaning ENABLE_SENSITIVE_DATA=true in the environment will no longer be overidden, potentially leaking sensitive telemetry data in production.

✓ Security Reliability

The PR correctly changes the default for enable_instrumentation from False to True, with sound env-var parsing logic. Sensitive data capture remains opt-in. All existing references to enable_instrumentation outside the diff are either pytest fixtures or attribute-level monkeypatches on OBSERVABILITY_SETTINGS (not calls to the removed function), so they won't break. The six existing unresolved review comments already cover the significant concerns: the breaking removal of enable_instrumentation() from the public API, the configure_azure_monitor regression when enable_sensitive_data=False, the DevUI boolean parsing inconsistency, the deployment modal comment mismatch, and the DevUI serve() parameter removal. No additional security or reliability defects were found.

✓ Test Coverage

The PR renames enable_instrumentation() to enable_sensitive_telemetry() and changes the default for enable_instrumentation from False to True. Test renames and adaptations are correct, but there are two notable test coverage gaps: (1) no test verifies the new default enable_instrumentation=True when the ENABLE_INSTRUMENTATION env var is unset — this is the core behavioral change of the PR, and (2) neither foundry test file covers the configure_azure_monitor(enable_sensitive_data=False) path, leaving the new if enable_sensitive_data: guard untested for the false branch.

✗ Design Approach

The main design issue is in the Agent Lightning integration: this PR removes the explicit instrumentation enablement from AgentFrameworkTracer.init(), which means that a user who has ENABLE_INSTRUMENTATION=false will no longer get Agent Framework spans through the Lightning tracer even though that tracer is documented as the component that enables Agent Framework observability for Lightning.

Flagged Issues

• configure_azure_monitor() no longer overrides ENABLE_SENSITIVE_DATA=true from the environment when called with the default enable_sensitive_data=False. The old code always called enable_instrumentation(enable_sensitive_data=False), which forcefully set OBSERVABILITY_SETTINGS.enable_sensitive_data=False. The new if enable_sensitive_data: guard skips the call entirely, leaving env-sourced enable_sensitive_data=True in effect. This is a security regression: sensitive data (prompts, tool arguments) may leak in production when users expect configure_azure_monitor() to suppress it.
• AgentFrameworkTracer no longer guarantees Agent Framework telemetry is enabled for the Lightning integration. With ENABLE_INSTRUMENTATION=false, observability gates become no-ops, which contradicts the Lightning tracer's documented contract to enable OpenTelemetry observability for Agent-lightning and the README guidance that instantiating AgentFrameworkTracer() sends telemetry to Lightning.

---

Automated review by TaoChenOSU's agents

[github-actions]

Automated Code Review

Reviewers: 4 | Confidence: 90%

✓ Correctness

The PR correctly changes the default for enable_instrumentation from False to True, adds a new enable_sensitive_telemetry() convenience function, gates expensive _capture_messages serialization on span.is_recording() as a performance optimization, and replaces assert_never in the converter with a graceful fallback. All code changes are well-tested and semantically correct. The _chat_client.py reformatting is semantically equivalent. No correctness bugs found.

✓ Security Reliability

This PR enables instrumentation by default, adds a new enable_sensitive_telemetry() convenience API, and gates expensive _capture_messages serialization on span.is_recording(). From a security and reliability perspective, the changes are sound: sensitive data capture (enable_sensitive_data) remains off by default and is gated behind SENSITIVE_DATA_ENABLED (which requires both enable_instrumentation AND enable_sensitive_data to be true). The span.is_recording() guards are a good reliability improvement that avoids unnecessary JSON serialization when no tracer provider is configured. The assert_never → logger.debug + skip change in _converter.py trades strict type exhaustiveness for forward compatibility, which is a reasonable tradeoff. No security or reliability issues found.

✓ Test Coverage

The PR's headline behavioral change — defaulting enable_instrumentation to True — lacks a direct unit test on ObservabilitySettings. The removed test_enable_instrumentation_reads_env_sensitive_data test leaves the env-var re-read fallback path of enable_instrumentation() without coverage. The new warning logic in __init__ is also untested. The span.is_recording() gating and the new enable_sensitive_telemetry() function are well-covered by the new tests.

✓ Design Approach

One design issue stands out. The ChatKit converter now silently drops any unsupported ThreadItem variant instead of surfacing that the converter no longer understands the thread payload. That conflicts with the converter’s own documented contract and can truncate conversation state without any caller-visible failure. I did not find another repo-backed design issue in the observability changes.

---

Automated review by TaoChenOSU's agents

[github-actions]

The PR's primary behavioral change (defaulting enable_instrumentation to True when the env var is unset) has no direct test. test_observability_settings_is_setup_initial deletes ENABLE_INSTRUMENTATION and creates ObservabilitySettings() but only asserts is_setup is False. Consider adding:

def test_observability_settings_defaults_instrumentation_true(monkeypatch):
    from agent_framework.observability import ObservabilitySettings
    monkeypatch.delenv("ENABLE_INSTRUMENTATION", raising=False)
    settings = ObservabilitySettings()
    assert settings.enable_instrumentation is True

[github-actions]

The removed test_enable_instrumentation_reads_env_sensitive_data was the only test covering the fallback branch where enable_sensitive_data is None and the env var ENABLE_SENSITIVE_DATA is "true" at call time. The replacement tests only cover the explicit-param path. Consider adding a sibling test that sets ENABLE_SENSITIVE_DATA=true after reload and calls enable_instrumentation() with no args, then asserts enable_sensitive_data is True.