-
Notifications
You must be signed in to change notification settings - Fork 777
feat(qdrant): Add query_batch_points, query_points and query_points_groups semantic conventions #3227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…roups semantic conventions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 3b6ec7a in 49 seconds. Click for details.
- Reviewed
37
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
3
draft 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-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py:239
- Draft comment:
New Qdrant query attributes added look consistent. Consider adding brief inline comments/docstrings to clarify their purpose. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/version.py:1
- Draft comment:
Version bump to 0.4.13 is correct. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. packages/opentelemetry-semantic-conventions-ai/pyproject.toml:11
- Draft comment:
pyproject.toml version updated to match version (0.4.13). - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_nfqV98J50xVK7N0Y
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
WalkthroughFour new Qdrant-related span attribute constants were added to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
243-246
: Maintain alphabetical order within the Qdrant section.The block is now:
QDRANT_SEARCH_... QDRANT_QUERY_... QDRANT_UPLOAD_...
Placing the new
QUERY_*
constants after allSEARCH_*
and beforeUPLOAD_*
preserves the current alphabetical grouping and simplifies future diffs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py
(1 hunks)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/version.py
(1 hunks)packages/opentelemetry-semantic-conventions-ai/pyproject.toml
(1 hunks)
🔇 Additional comments (2)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/version.py (1)
1-1
: Version bump looks correct and consistent withpyproject.toml
.No further concerns.
packages/opentelemetry-semantic-conventions-ai/pyproject.toml (1)
11-11
: Package version updated — good.The Poetry version matches
version.__version__
; nothing else to change.
QDRANT_QUERY_POINTS_COLLECTION_NAME = "qdrant.query_points.collection_name" | ||
QDRANT_QUERY_POINTS_GROUPS_COLLECTION_NAME = "qdrant.query_points_groups.collection_name" | ||
QDRANT_QUERY_BATCH_POINTS_COLLECTION_NAME = "qdrant.query_batch_points.collection_name" | ||
QDRANT_QUERY_BATCH_POINTS_REQUESTS_COUNT = "qdrant.query_batch_points.requests_count" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Align new Qdrant attribute names with existing naming scheme.
Existing Qdrant attributes use the pattern qdrant.<operation>[_batch].collection_name
/ ...requests_count
.
The newly-added constants introduce an extra “ points ” token, e.g. query_batch_points
, which breaks that pattern and will sort far away from the related attributes in dashboards.
Consider renaming to keep the surface uniform:
-QDRANT_QUERY_POINTS_COLLECTION_NAME = "qdrant.query_points.collection_name"
-QDRANT_QUERY_POINTS_GROUPS_COLLECTION_NAME = "qdrant.query_points_groups.collection_name"
-QDRANT_QUERY_BATCH_POINTS_COLLECTION_NAME = "qdrant.query_batch_points.collection_name"
-QDRANT_QUERY_BATCH_POINTS_REQUESTS_COUNT = "qdrant.query_batch_points.requests_count"
+QDRANT_QUERY_COLLECTION_NAME = "qdrant.query.collection_name"
+QDRANT_QUERY_GROUPS_COLLECTION_NAME = "qdrant.query_groups.collection_name"
+QDRANT_QUERY_BATCH_COLLECTION_NAME = "qdrant.query_batch.collection_name"
+QDRANT_QUERY_BATCH_REQUESTS_COUNT = "qdrant.query_batch.requests_count"
This keeps “ query / query_batch / query_groups ” parallel to “ search / search_batch ”.
If the “points” wording is required by Qdrant’s API docs, at minimum add a short comment explaining the deviation to avoid confusion.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
QDRANT_QUERY_POINTS_COLLECTION_NAME = "qdrant.query_points.collection_name" | |
QDRANT_QUERY_POINTS_GROUPS_COLLECTION_NAME = "qdrant.query_points_groups.collection_name" | |
QDRANT_QUERY_BATCH_POINTS_COLLECTION_NAME = "qdrant.query_batch_points.collection_name" | |
QDRANT_QUERY_BATCH_POINTS_REQUESTS_COUNT = "qdrant.query_batch_points.requests_count" | |
QDRANT_QUERY_COLLECTION_NAME = "qdrant.query.collection_name" | |
QDRANT_QUERY_GROUPS_COLLECTION_NAME = "qdrant.query_groups.collection_name" | |
QDRANT_QUERY_BATCH_COLLECTION_NAME = "qdrant.query_batch.collection_name" | |
QDRANT_QUERY_BATCH_REQUESTS_COUNT = "qdrant.query_batch.requests_count" |
🤖 Prompt for AI Agents
In
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py
around lines 243 to 246, the new Qdrant attribute names include an extra
"points" token that breaks the existing naming pattern and affects sorting in
dashboards. Rename these constants to remove "points" so they follow the
established pattern like qdrant.query.collection_name,
qdrant.query_groups.collection_name, and qdrant.query_batch.collection_name. If
the "points" term is required by Qdrant's API, add a brief comment explaining
this exception to maintain clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duynguyenhoang why not re-use the existing semconvs?
@nirga, qdrant introduced new methods and we are going to instrument them
It makes sense to add new span attributes, instead of reuse the current. With current package dependencies, I need to separate the MR into 2. |
First MR to add new qdrant sematic conventions.
feat(instrumentation): ...
orfix(instrumentation): ...
.Important
Add new Qdrant semantic conventions to
SpanAttributes
and update version to0.4.13
.QDRANT_QUERY_POINTS_COLLECTION_NAME
,QDRANT_QUERY_POINTS_GROUPS_COLLECTION_NAME
,QDRANT_QUERY_BATCH_POINTS_COLLECTION_NAME
, andQDRANT_QUERY_BATCH_POINTS_REQUESTS_COUNT
toSpanAttributes
in__init__.py
.0.4.13
inversion.py
andpyproject.toml
.This description was created by
for 3b6ec7a. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit