-
Notifications
You must be signed in to change notification settings - Fork 2k
Enhance vector store observability support #1262
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
Enhance vector store observability support #1262
Conversation
ThomasVitale
commented
Aug 21, 2024
- Consolidate usage of “db.collection.name” attribute to track table name, collection name, index name, document name, or whatever concept a vector database uses to store data. Removed “db.index” that was use sometimes instead of “db.collection.name”. This usage is in line with the OpenTelemetry Semantic Conventions.
- Configure query response content to be included as a “span event” instead of a “span attribute” if the backend system supports that, similar to how we do for the model observations.
- Structure vector store observation attributes in dedicated enums, including one for the Spring AI Kinds to avoid hard-coding the same value in a lot of places. This follows the OpenTelemetry Semantic Conventions as much as possible. Also, adopt Spring usual non-null-by-default strategy as much as possible.
- Align vector store conventions to the chat model ones, and follow alphabetical order for values. This is particularly useful for the convention classes, for which the Micrometer performance of exporting telemetry data improves when key values are added already sorted to the context.
- Fix flaky test in Mistral AI.
- Improve Qdrant integration tests.
29139ee
to
a5fd8ed
Compare
.hasHighCardinalityKeyValue(HighCardinalityKeyNames.REQUEST_TOP_K.asString(), KeyValue.NONE_VALUE) | ||
.hasHighCardinalityKeyValue(HighCardinalityKeyNames.REQUEST_TOP_P.asString(), "1.0") | ||
.hasHighCardinalityKeyValue(HighCardinalityKeyNames.RESPONSE_ID.asString(), responseMetadata.getId()) | ||
.hasHighCardinalityKeyValue(HighCardinalityKeyNames.RESPONSE_ID.asString(), |
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.
This was flaky, Mistral AI doesn't always return a Response ID in streaming mode.
chatModelObservationContext | ||
.addHighCardinalityKeyValue(ChatModelObservationDocumentation.HighCardinalityKeyNames.COMPLETION | ||
.withValue(ChatModelObservationContentProcessor.concatenateStrings(completions))); | ||
.withValue(TracingHelper.concatenateStrings(completions))); |
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.
Extracted the logic into the TracingHelper
to re-use it for the vector store observations.
/** | ||
* The name of the operation or command being executed. | ||
*/ | ||
DB_OPERATION_NAME("db.operation.name"),; |
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.
Moved to VectorStoreObservationAttributes
|
||
// @formatter:off | ||
|
||
CHAT_CLIENT("chat_client"), |
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.
The first two values here are not used yet, I didn't want to make the PR even bigger. We can adopt them in a separate PR.
import io.micrometer.common.KeyValues; | ||
|
||
/** | ||
* Default conventions to populate observations for vector store operations. |
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.
The main change here is a resorting to feed Micrometer already sorted list of key values for a performance improvement, and updating the observation name to be closer to the semantic conventions
* Consolidate usage of “db.collection.name” attribute to track table name, collection name, index name, document name, or whatever concept a vector database uses to store data. Removed “db.index” that was use sometimes instead of “db.collection.name”. This usage is in line with the OpenTelemetry Semantic Conventions. * Configure query response content to be included as a “span event” instead of a “span attribute” if the backend system supports that, similar to how we do for the model observations. * Structure vector store observation attributes in dedicated enums, including one for the Spring AI Kinds to avoid hard-coding the same value in a lot of places. This follows the OpenTelemetry Semantic Conventions as much as possible. Also, adopt Spring usual non-null-by-default strategy as much as possible. * Align vector store conventions to the chat model ones, and follow alphabetical order for values. This is particularly useful for the convention classes, for which the Micrometer performance of exporting telemetry data improves when key values are added already sorted to the context. * Fix flaky test in Mistral AI. * Improve Qdrant integration tests. Signed-off-by: Thomas Vitale <[email protected]>
a5fd8ed
to
89ffd68
Compare
I've added .withCollectionName(this.pineconeIndexName) to the PineconeVectorStore |
Rebased and merged at 036093a |