Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Wrapper selection and span recording packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py, packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py |
The instrumentor selects _awrap for coroutine methods and _wrap for other methods. Both wrappers use shared request-attribute logic. _awrap awaits the operation inside a Qdrant span. |
Async instrumentation validation packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_async_instrumentation.py |
Tests cover span timing and parentage, request errors, cancellation, suppression, unawaited calls, empty results, upload methods, and restoration after uninstrumentation. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Low
Sequence Diagram(s)
sequenceDiagram
participant Caller
participant QdrantInstrumentor
participant AsyncQdrantClient
participant _awrap
participant HTTPTransport
QdrantInstrumentor->>AsyncQdrantClient: wraps coroutine method with _awrap
Caller->>AsyncQdrantClient: awaits Qdrant operation
AsyncQdrantClient->>_awrap: invokes wrapped method
_awrap->>HTTPTransport: awaits operation inside Qdrant span
HTTPTransport-->>_awrap: returns response or raises error
_awrap-->>Caller: returns response or propagates error
Possibly related issues
- traceloop/openllmetry#4658: The changes address the reported async span lifecycle, parentage, and awaited exception behavior.
Suggested reviewers: anush008
Merge Risk: 
8525f
-
No identified issue currently blocks merging the Qdrant async span fix, subject to normal checks.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the main change: Qdrant async instrumentation spans now stay open until the awaited operation completes. The raw summary confirms an async wrapper awaits the operation inside start_as_… Linked Issues check Issue [ #4658] is the only active directly linked issue. The implementation selects_awrapfor coroutine methods and_wrapfor synchronous methods._awrapawaits the Qdrant operation inside `star…Out of Scope Changes check The changes stay within issue [ #4658]. The wrapper-selection logic addresses mixed synchronous and asynchronous methods onAsyncQdrantClient._set_request_attributesremoves duplicated attribute d…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 3 files. Full details: Title check
-
Explanation
The title names the main change: Qdrant async instrumentation spans now stay open until the awaited operation completes. The raw summary confirms an async wrapper awaits the operation inside start_as_current_span, and the linked issue
#4658asks for the same span lifecycle fix. The title is specific, uses a conventional commit prefix, and is 67 characters long, which is within a reasonable length.
Full details: Linked Issues check
-
Explanation
Issue [
#4658] is the only active directly linked issue. The implementation selects_awrapfor coroutine methods and_wrapfor synchronous methods._awrapawaits the Qdrant operation insidestart_as_current_span, so the span remains current until completion. Exceptions during the await pass through the span context and receive ERROR status with an exception event. The coroutine wrapper starts no span before the coroutine is awaited. The new tests cover pending requests, parentage, errors, cancellation, unawaited calls, suppression, synchronous upload methods, and uninstrumentation.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue [
#4658]. The wrapper-selection logic addresses mixed synchronous and asynchronous methods onAsyncQdrantClient._set_request_attributesremoves duplicated attribute dispatch while preserving the existing method coverage and attribute names. The regression tests verify the requested lifecycle behavior. Added docstrings describe the changed instrumentation code. No unrelated API, dependency, configuration, method-coverage, or compatibility changes are present.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
|
9fd6118 to
048a394
Compare
Fixes #4658
Summary
AsyncQdrantClient.upsert()currently ends its Qdrant span as soon as the client returns a coroutine. The actual request then runs outside that span: durations are nearly zero, transport spans inherit the caller, and exceptions raised duringawaitleave an OK span.Add an async wrapper that awaits the operation inside
start_as_current_span. Choose the wrapper using the actual method's coroutine status, sinceAsyncQdrantClientalso exposes synchronous upload methods. Share request-attribute handling between the sync and async wrappers.Span names, attributes, method coverage, and public APIs stay the same. No documentation or configuration change is needed.
Tests
The upstream lockfile is already stale:
uv lock --checkfails on the unchanged base. Local validation used temporary environments resolved from the existing pyproject constraints. This PR does not change dependencies or lockfiles.Screenshots
Captured from Jaeger 2.6.0 using the real client and instrumentor with
httpx.MockTransportand a controlled 250 ms delay. These show instrumentation lifecycle behavior, not live Qdrant server latency. The standalone reproducer is included in #4658.Successful request: before and after
Before: the Qdrant span lasts 15 microseconds; the 251 ms transport span is its sibling.
After: the Qdrant span lasts 262.125 ms and parents the transport span.
HTTP 500: before and after
Before: the Qdrant span lasts 14 microseconds, remains OK, and has no exception event.
After: the Qdrant span lasts 261.740 ms, has status ERROR, and records one exception event.
feat(instrumentation): ...orfix(instrumentation): ....Summary by CodeRabbit