fix(qdrant): support all versions of qdrant package#3500
fix(qdrant): support all versions of qdrant package#3500nina-kollman merged 8 commits intotraceloop:mainfrom
Conversation
WalkthroughUpdates add support for QdrantClient.query_points and query_batch_points, extend wrapper attribute handling with backward-compatible naming, add safer instrumentation initialization with exception handling, update tests to new APIs, and add a sample app demonstrating query_points usage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to d096021 in 1 minute and 25 seconds. Click for details.
- Reviewed
212lines of code in5files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py:54
- Draft comment:
Good defensive wrapping with try/except; consider using logger.exception (or including the traceback) to help with debugging instrumentation failures. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/qdrant_client_methods.json:47
- Draft comment:
New methods 'query_points' and 'query_batch_points' have been added. Ensure these names match the actual Qdrant client API and that any breaking changes are documented. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py:64
- Draft comment:
The wrapper now includes 'query_points' and 'query_batch_points' (with backward-compatible mapping to 'search' and 'search_batch'). Consider updating docstrings or external docs to reflect this behavior. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py:80
- Draft comment:
Test updates now invoke 'query_points' and 'query_batch_points'. Verify if legacy search methods should also retain tests for backward compatibility. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. packages/sample-app/sample_app/qdrant_app.py:47
- Draft comment:
Sample app correctly uses the new 'query_points' method. For style consistency, please ensure the file ends with a newline. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py:87
- Draft comment:
Typo noticed: The function name 'test_qdrant_searchs' may be incorrect (possibly intended to be 'test_qdrant_search' or 'test_qdrant_searches'). Please confirm the intended naming. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_gaDPKGYPVWlPX25K
You can customize 
elinacse
changed the title
|
@nirga @gyliu513 I have submitted this PR for the issue FIx #3492 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py (1)
54-68: Narrow the exception handling to specific exception types.Catching broad
Exceptioncan mask unexpected errors. Consider catching specific exceptions thatwrap_function_wrappermight raise, such asAttributeError,ImportError, orValueError.Apply this diff:
target_class = getattr(qdrant_client, wrap_object, None) if target_class and hasattr(target_class, wrap_method): try: wrap_function_wrapper( MODULE, f"{wrap_object}.{wrap_method}", _wrap(tracer, wrapped_method), ) - except Exception as e: + except (AttributeError, ImportError, ValueError) as e: logger.warning( "Failed to instrument %s.%s: %s", wrap_object, wrap_method, str(e), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py(1 hunks)packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/qdrant_client_methods.json(1 hunks)packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py(3 hunks)packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py(3 hunks)packages/sample-app/sample_app/qdrant_app.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.pypackages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.pypackages/sample-app/sample_app/qdrant_app.pypackages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py
🧬 Code graph analysis (3)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py (2)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py (1)
_wrap(31-76)packages/opentelemetry-instrumentation-weaviate/opentelemetry/instrumentation/weaviate/wrapper.py (1)
_wrap(34-51)
packages/sample-app/sample_app/qdrant_app.py (1)
packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(36-267)init(48-198)
packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py (2)
packages/opentelemetry-instrumentation-qdrant/tests/conftest.py (1)
exporter(12-22)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
🪛 Flake8 (7.3.0)
packages/sample-app/sample_app/qdrant_app.py
[error] 4-4: 'numpy as np' imported but unused
(F401)
🪛 Ruff (0.14.7)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py
62-62: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (3)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/qdrant_client_methods.json (1)
47-51: LGTM!The new method mappings for
query_pointsandquery_batch_pointsare properly structured and consistent with the existing entries.Also applies to: 57-61
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py (1)
64-64: LGTM! Excellent backward compatibility approach.The additions of
query_pointsandquery_batch_pointsare well-implemented with backward-compatible attribute naming. This ensures existing monitoring and alerting systems continue to work while the span names reflect the new API methods.Also applies to: 70-70, 81-92, 122-124
packages/opentelemetry-instrumentation-qdrant/tests/test_qdrant_instrumentation.py (1)
80-98: LGTM! Tests properly validate new APIs and backward compatibility.The test updates correctly exercise the new
query_pointsandquery_batch_pointsmethods with updated models (QueryRequest), while importantly verifying that the backward-compatible attribute names (e.g.,QDRANT_SEARCH_COLLECTION_NAME) are still populated correctly.Also applies to: 102-124
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py (1)
60-71: Wiringquery_points/query_batch_pointsinto existing search paths looks correctIncluding
query_pointsin the search-like branch andquery_batch_pointsin the batch-search branch cleanly reuses the existing attribute logic without changing behavior for older methods. This is a straightforward, compatible extension.If you want to keep style consistent with the multi-line list above, you could also break the line at Line 70 into a multi-line list, but that’s purely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py(3 hunks)packages/sample-app/sample_app/qdrant_app.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sample-app/sample_app/qdrant_app.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py (6)
packages/opentelemetry-instrumentation-milvus/opentelemetry/instrumentation/milvus/wrapper.py (2)
_set_search_attributes(280-328)_set_span_attribute(53-57)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/__init__.py (1)
_set_span_attribute(31-38)packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.py (1)
_set_span_attribute(105-109)packages/opentelemetry-instrumentation-llamaindex/opentelemetry/instrumentation/llamaindex/custom_llm_instrumentor.py (1)
_set_span_attribute(71-75)packages/opentelemetry-instrumentation-chromadb/opentelemetry/instrumentation/chromadb/wrapper.py (1)
_set_span_attribute(26-30)packages/opentelemetry-instrumentation-weaviate/opentelemetry/instrumentation/weaviate/wrapper.py (1)
_set_span_attribute(26-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/wrapper.py (2)
79-91: Backward-compatibleattribute_methodmapping is consistent and avoids new attribute keysUsing
attribute_methodto map:
query_points→searchquery_batch_points→search_batchbefore constructing
qdrant.{attribute_method}.collection_namekeeps all collection-name attributes under the existingqdrant.search.*/qdrant.search_batch.*namespaces, which helps avoid fragmenting metrics across old and new method names.This looks correct and consistent with the intent of supporting newer client APIs without breaking existing dashboards.
119-123: Batch search attribute mapping forquery_batch_pointsmatches collection-name behaviorDefaulting
requeststo an empty list and then mapping:
attribute_method = "search_batch"whenmethod == "query_batch_points"so that you emit
qdrant.search_batch.requests_countkeeps batch request-count metrics aligned with the priorsearch_batchnaming. This mirrors the mapping in_set_collection_name_attributeand maintains backward compatibility without changing behavior for other batch methods.No issues here.
nirga
changed the title
|
@nirga , I don't see the PR merge option , Could you please help merging the PR to main branch. Thanks ! |
|
@nirga Could you please help merge this PR . Thanks ! |
|
@nirga Could you please help merge this PR . Thanks ! |
|
@elinacse I guess you should also update the project version and qdrant-client dependency in pyproject.toml |
Co-authored-by: Nir Gazit <nirga@users.noreply.github.com>
Co-authored-by: Nir Gazit <nirga@users.noreply.github.com>
4 participants
feat(instrumentation): ...orfix(instrumentation): ....Important
Enhance Qdrant instrumentation by adding
query_pointsandquery_batch_pointsmethods, updating tests, and adding a sample app.query_pointsandquery_batch_pointsmethods toqdrant_client_methods.json._instrument()in__init__.pyto handle new methods and log errors if instrumentation fails._wrap()inwrapper.pyto support new methods and map them for backward compatibility.test_qdrant_instrumentation.pyto testquery_pointsandquery_batch_pointsmethods.qdrant_app.pyto demonstrate usage ofquery_pointswith a sample dataset.This description was created by
for d096021. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.