[Hardik K (hardikkaurani)]

Summary

Encloses downstream execution (chain.proceed) within the active span lifecycle in UserAgentHandler and UrlReplaceHandler and propagates the handler span onto the outgoing request tag (Span.class) so child spans remain properly nested within the tracing hierarchy.

Fix

• UserAgentHandler.java:
  • Moved chain.proceed(builder.build()) into the try block before the finally block, ensuring span.end() and scope.close() execute only after downstream execution completes.
  • Tagged Span.class on the outgoing request (builder.tag(Span.class, span)), enabling downstream middleware and transport calls to attach as child spans.
• UrlReplaceHandler.java:
  • Moved chain.proceed(request) into the try block before the finally block, ensuring span.end() and scope.close() accurately measure total duration including downstream transport.
  • Tagged Span.class on the outgoing request (request.newBuilder().tag(Span.class, span).build()).
• Unit Tests:
  • UserAgentHandlerTest.java: Added enclosesProceedAndPropagatesSpan verifying downstream receives the tagged span.
  • UrlReplaceHandlerTest.java: Added testUrlReplaceHandler_propagatesSpan verifying span propagation to chain.proceed.

Testing

• Ran targeted okHttp middleware unit tests:

  ./gradlew :components:http:okHttp:test --tests "com.microsoft.kiota.http.middleware.UserAgentHandlerTest" --tests "com.microsoft.kiota.http.middleware.UrlReplaceHandlerTest"
  

  Result: BUILD SUCCESSFUL (all 9 tests passed).
• Ran full okHttp test suite:

  ./gradlew :components:http:okHttp:test
  

  Result: BUILD SUCCESSFUL (all tests passed across the okHttp module).

cc Vincent Biret (@baywet)

[Copilot]

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

[Hardik K (hardikkaurani)]

Hi Vincent Biret (@baywet),

Following up on our middleware work in okHttp, this PR aligns the OpenTelemetry span lifecycle in UserAgentHandler and UrlReplaceHandler:

• Encloses downstream execution (chain.proceed) within the ry/finally block so span.end() and scope.close() accurately measure full request duration.
• Tags Span.class onto outgoing request builders so downstream operations and child spans nest properly under the handler span.

All unit tests and the full okHttp module test suite (./gradlew :components:http:okHttp:test) pass cleanly locally.

Whenever convenient, please take a look.

[Copilot]

🟡 Changes recommended

Both regression tests verify span tagging but leave the central lifecycle correction unprotected against regression.

2 open issues

• Add success and IOException assertions for handler span lifecycle · New
• Strengthen test to verify span lifecycle during and after proceed · New

_{Review effort: Balanced}

[Copilot]

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

[Hardik K (hardikkaurani)]

Hi Vincent Biret (@baywet),

I've updated the PR with strengthened regression tests for both UserAgentHandler and UrlReplaceHandler:

1. Span & Scope lifecycle verification: Extended tests with controllable Tracer/Span/Scope mocking to verify that during chain.proceed, the handler span is tagged on the outgoing request, remains active, and is not ended prematurely.
2. Context restoration: Asserted that upon request completion, scope.close() and span.end() are called in the finally block to end the span and restore the previous context.
3. Failure path coverage: Added test cases verifying that cleanup (scope.close() and span.end()) executes reliably even when downstream execution throws an IOException.

All okHttp tests pass cleanly (BUILD SUCCESSFUL), and Spotless / SpotBugs checks are green. Whenever you have time, please take another look!

[Copilot]

🟢 Approval recommended

The focused lifecycle and propagation changes have regression coverage for success and failure paths, with no unresolved blocking issues.

No open issues 🎉

2 resolved since last review

• Strengthen test to verify span lifecycle during and after proceed
• Add success and IOException assertions for handler span lifecycle

_{Review effort: Balanced}