Skip to content

fix(http): isolate parent span per request, extend user agent span over transport, and record telemetry errors - #827

Open
Hardik K (hardikkaurani) wants to merge 1 commit into
microsoft:mainfrom
hardikkaurani:fix/middleware-span-lifecycle-and-parent-isolation
Open

Hardik K (hardikkaurani) wants to merge 1 commit into
microsoft:mainfrom
hardikkaurani:fix/middleware-span-lifecycle-and-parent-isolation

Conversation

@hardikkaurani

Copy link
Copy Markdown
Contributor

Summary

Addresses OpenTelemetry span lifecycle and isolation in kiota_http:

  1. Encloses downstream execution in UserAgentHandler within the handler span lifecycle (try...finally: _span.end()) and records downstream pipeline errors via _span.record_exception(exc).
  2. Prevents per-request parent_span options from mutating BaseMiddleware.parent_span on the middleware instance, isolating trace contexts between requests.
  3. Adds defensive null request checks and telemetry exception recording to UrlReplaceHandler and UserAgentHandler.

Fix

  • middleware.py (BaseMiddleware._create_observability_span):
    • Bound per-request parent_span locally instead of mutating self.parent_span. Subsequent requests processed by the same middleware instance without a parent_span option no longer inherit earlier trace spans.
  • user_agent_handler.py (UserAgentHandler.send):
    • Added null check if request is None: raise TypeError("request cannot be null").
    • Moved _span.end() into a finally block enclosing await super().send(request, transport) so the span accurately measures full execution and downstream spans remain properly nested.
    • Added exception handling to record errors on the span before re-raising (_span.record_exception(exc)).
  • url_replace_handler.py (UrlReplaceHandler.send):
    • Added null check if request is None: raise TypeError("request cannot be null").
    • Added exception handling to record errors on the span before re-raising (_enable_span.record_exception(exc)).
  • Unit Tests:
    • test_base_middleware.py: Added test_parent_span_option_does_not_mutate_middleware_instance verifying request options do not leak parent spans across requests.
    • test_user_agent_handler.py: Added tests verifying null request rejection, span enclosing transport, and exception recording on transport failure.
    • test_url_replace_handler.py: Added tests verifying null request rejection and exception recording on transport failure.

Testing

  • Ran all targeted middleware tests:
    pytest packages/http/httpx/tests/middleware_tests/test_user_agent_handler.py packages/http/httpx/tests/middleware_tests/test_url_replace_handler.py packages/http/httpx/tests/middleware_tests/test_base_middleware.py
    
    Result: 21/21 tests PASS in 0.12s.
  • Ran full HTTP test suite:
    pytest packages/http/httpx/tests
    
    Result: 205/205 tests PASS in 68.90s.

cc Vincent Biret (@baywet)

@hardikkaurani
Hardik K (hardikkaurani) requested a review from a team as a code owner October 24, 2026 10:38
Copilot AI balanced review requested due to automatic review settings October 24, 2026 10:38
@hardikkaurani

Copy link
Copy Markdown
Contributor Author

Hi Vincent Biret (@baywet),

Following up on our middleware work in kiota_http, this PR addresses OpenTelemetry span lifecycle and isolation:

  • Encloses UserAgentHandler.send within the span lifecycle ( ry...finally: _span.end()) so downstream operations remain nested within the handler span, and records pipeline exceptions via _span.record_exception(exc).
  • Prevents per-request parent_span options from mutating BaseMiddleware.parent_span on the middleware instance, ensuring trace contexts remain strictly isolated between requests.
  • Adds defensive null request checks and telemetry exception recording to UrlReplaceHandler and UserAgentHandler.

All new unit tests and the full HTTP test suite (pytest packages/http/httpx/tests) pass cleanly locally (205 passed in 68.90s).

Whenever convenient, please take a look.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Transport cleanup removes the explicit request parent before retry-attempt and subsequent downstream spans can reuse it.

1 open issue

Review effort: Balanced

Comment on lines +72 to +75
parent_span = self.parent_span
if options and (req_parent_span := options.get("parent_span", None)):
parent_span = req_parent_span
if parent_span is None:

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants