Skip to content

Python: record gen_ai.client.operation.duration for failed chat and embedding calls - #8347

Merged
Evan Mattson (moonbox3) merged 4 commits into
microsoft:mainfrom
rksharma-owg:fix/7818-operation-duration-on-failure
Sep 15, 2026
Merged

Evan Mattson (moonbox3) merged 4 commits into
microsoft:mainfrom
rksharma-owg:fix/7818-operation-duration-on-failure

Conversation

@rksharma-owg

Copy link
Copy Markdown
Contributor

Motivation & Context

OpenTelemetry Semantic Conventions for Generative AI operations define gen_ai.client.operation.duration for failed operations as well as successful ones, with error.type set to the class of the error. Recording only successes leaves error latency out of the metric and prevents calculating error rates from operation duration histograms.

This resolves #7818, incorporating previous feedback from Eduard van Valkenburg (@eavanvalkenburg) by centralizing GenAI metric attribute filtering and adding tests for synchronous chat, streaming setup/iteration, and embedding failure paths.

Description & Review Guide

  • What are the major changes?
    • Added helper _filter_metric_attributes to project GEN_AI_METRIC_ATTRIBUTES in a single place.
    • Added _capture_operation_error to record operation_duration_histogram on failures with error.type.
    • Updated ChatTelemetryLayer (non-streaming, streaming setup, and streaming iteration) to record operation duration on error before closing/raising.
    • Updated EmbeddingTelemetryLayer to record operation duration on error.
    • Added unit test coverage for each failure path and attribute projection.
  • What is the impact of these changes?
    • Generative AI client operation duration metrics are accurately recorded on failure, restoring parity with OpenTelemetry semantic conventions.
  • What do you want reviewers to focus on?
    • Attribute filtering consistency and error-path instrumentation in ChatTelemetryLayer and EmbeddingTelemetryLayer.

Related Issue

Fixes #7818

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Streaming finalizer and result-hook failures still omit the operation-duration metric.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds failed-operation duration metrics for Python chat and embedding telemetry.

Changes:

  • Centralizes GenAI metric attribute filtering.
  • Records duration and error.type for chat and embedding failures.
  • Adds failure-path unit tests.
File summaries
File Description
python/packages/core/agent_framework/observability.py Adds error-duration metric capture.
python/packages/core/tests/core/test_observability.py Tests filtering and failure paths.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/agent_framework/observability.py
Comment thread python/packages/core/agent_framework/observability.py Outdated
…ze_stream

Narrow error handling to get_final_response and re-raise exceptions while independently guarding telemetry capture, ensuring caller outcomes and duration error metrics remain aligned.
@eavanvalkenburg

Copy link
Copy Markdown
Member

Thanks for the update. Before this is ready, could you please:

Once those are addressed, please re-request review. Thanks!

@rksharma-owg

Copy link
Copy Markdown
Contributor Author

Eduard van Valkenburg (@eavanvalkenburg) Resolved the review discussion thread and addressed the feedback in commit 465198a (re-raising exceptions on get_final_response() and independently guarding telemetry capture). Ready for re-review. Thanks!

@moonbox3

Copy link
Copy Markdown
Contributor

Please fix the failing CI/CD checks.

@rksharma-owg

Copy link
Copy Markdown
Contributor Author

The failing Hyperlight cleanup test was racing background directory removal. I merged upstream main in 3cc91b8, which includes the fix from #8380.

Preflight CI on this exact commit passed all 10 Linux/Windows unit-test jobs across Python 3.10–3.14. The Ubuntu/Python 3.11 coverage check also passed: 91.1% overall, with every enforced package meeting the 85% threshold.

The new upstream workflows are awaiting approval, so their results are still pending. Could a maintainer approve those runs?

@moonbox3
Evan Mattson (moonbox3) added this pull request to the merge queue Sep 15, 2026
Merged via the queue into microsoft:main with commit a757aeb Sep 15, 2026
40 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth 3cc91b83 Deployed Sep 15, 2026 by rksharma-owg via add_label #22838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: gen_ai.client.operation.duration is never recorded for failed chat or embedding calls

4 participants