Add observability support to VectorStore #1205

tzolov · 2024-08-11T06:32:31Z

Introduce AbstractObservationVectorStore for instrumentation. Add observe instrumentation for the add, delete and the similaritySearch methods.
Add VectorStoreObservationContext for capturing operation details.
Implement DefaultVectorStoreObservationConvention for naming and tagging
Create filters for add and delete request content observation Implement VectorStoreQueryResponseContentObservationFilter.
Update VectorStore interface with getName() method, returning the class name by default.
Add VectorStoreObservationDocumentation for defining observation keys.
Create VectorStoreObservationAutoConfiguration for auto-configuring observations.
Add VectorStoreObservationProperties for configuring observation behavior. They are used to control the optional, additional, observationcontent filters.
Update PgVectorStoreAutoConfiguration to support observations.
Modify PgVectorStore to extend AbstractObservationVectorStore
Add observation support to PgVectorStore's Builder.

Resolves #1204

ThomasVitale · 2024-08-14T13:38:29Z

...java/org/springframework/ai/vectorstore/observation/VectorStoreObservationDocumentation.java

+		DB_OPERATION_NAME {
+			@Override
+			public String asString() {
+				return "db.operation.name";


Consider adding this attributes coming from the OTel Conventions (db.*) to the AiObservationAttributes enum.

ThomasVitale · 2024-08-14T13:41:56Z

...java/org/springframework/ai/vectorstore/observation/VectorStoreObservationDocumentation.java

+		QUERY {
+			@Override
+			public String asString() {
+				return "db.query.text";


I can see that this is used to add the text that is eventually translated into embeddings, and not the actual database query. Perhaps we should rename this to something else? How about db.vector.query.content? I expect the instrumentation for the lower-level database to use the db.query.text attribute for the db-specific query like SELECT *, embedding <=> ? AS distance FROM public.vector_store WHERE embedding <=> ? < ? ORDER BY distance LIMIT ? (getting this from PGVector when using the "net.ttddyy.observation:datasource-micrometer-spring-boot" library)

db.vector.query.content sounds quite reasonable to me.

ThomasVitale · 2024-08-14T13:50:46Z

...java/org/springframework/ai/vectorstore/observation/VectorStoreObservationDocumentation.java

+		DELETE_REQUEST {
+			@Override
+			public String asString() {
+				return "db.vector.delete.request";


Do we need to include the content of the documents we add/delete? I'm not sure about a possible use case for this data, it's typically not something included in database instrumentation. I can see it's also not included in LLM products like OpenLLMetry/TraceLoop and OpenLit.

The add/delete content is optional (e.g. requires filters) and is disabled by default.
My though was that if you want to debug/trace or provide providence for you app you might need to enable logging/trace this data as well.
But if you and Mark consider that this is too rare need I'm Ok to drop it?

ThomasVitale · 2024-08-14T13:54:34Z

...java/org/springframework/ai/vectorstore/observation/VectorStoreObservationDocumentation.java

+		QUERY_RESPONSE {
+			@Override
+			public String asString() {
+				return "db.vector.query.response";


If this is used for the documents returned in the response, I would suggest changing it to db.vector.query.response.documents, and establish a namespace (db.vector.query.response.*) we extend in the future to observe response metadata, response dimension, response vectors, and so on.

that sounds like a good idea.

ThomasVitale · 2024-08-14T13:56:58Z

...java/org/springframework/ai/vectorstore/observation/VectorStoreObservationDocumentation.java

+		QUERY_METADATA_FILTER {
+			@Override
+			public String asString() {
+				return "db.query.metadata.filter";


I would keep using the vector namespace here and name the attribute db.vector.query.metadata.filter or even just db.vector.query.filter

ThomasVitale · 2024-08-14T14:02:11Z

.../main/java/org/springframework/ai/vectorstore/observation/VectorStoreObservationContext.java

+	}
+
+	// DELETE
+	private List<String> deleteRequest;


This list of fields might grow a lot once Spring AI add support for more operations (like for updating embeddings for documents that changed). Consider if they can be grouped into some Records perhaps, or even maybe have a base VectorStoreObservationContext (that might be enough for add/delete operations if we remove the need to track the Documents) and specialised implementation whenever relevant (e.g. QueryVectorStoreObservationContext for semantic search)

Conventions and Documentations can remain the same, all grouped

Actually intended this initially but then seemed like unnecessary complexity because of the shared context attributes.
I will rethink it again.

ThomasVitale · 2024-08-14T14:07:03Z

...pringframework/ai/vectorstore/observation/VectorStoreAddRequestContentObservationFilter.java

+ * @author Christian Tzolov
+ * @since 1.0.0
+ */
+public class VectorStoreAddRequestContentObservationFilter implements ObservationFilter {


I would consider not having this and remove the related attribute, unless it's needed for some specific use case

same as #1205 (comment)

ThomasVitale · 2024-08-14T14:07:16Z

...ngframework/ai/vectorstore/observation/VectorStoreDeleteRequestContentObservationFilter.java

+ * @author Christian Tzolov
+ * @since 1.0.0
+ */
+public class VectorStoreDeleteRequestContentObservationFilter implements ObservationFilter {


Like for the "add" scenario, I would consider not having this and remove the related attribute, unless it's needed for some specific use case

same as #1205 (comment)

ThomasVitale · 2024-08-14T14:10:58Z

...spring-ai-pgvector-store/src/main/java/org/springframework/ai/vectorstore/PgVectorStore.java

+			.withDimensions(this.embeddingDimensions())
+			.withCollectionName(this.vectorTableName)
+			.withNamespace(this.schemaName)
+			.withSimilarityMetric(this.getDistanceType().name())


The OTel Conventions have defined a list of well known values for the similarity metric. I would suggest defining a VectorStoreSimilarityMetric enum in the same package as AiOperationType and use those values if possible. That will allow LLM observability backend to recognise the known values. Here, we would need to map between the DB specific name and the convention name.

See: https://github.com/open-telemetry/semantic-conventions/pull/1231/files#diff-c3afcbe6f6acbeb5582972b0c76d7aebe0cfc01313c0ed92919b2c18ffe76785R216

ThomasVitale · 2024-08-14T14:19:17Z

...spring-ai-pgvector-store/src/main/java/org/springframework/ai/vectorstore/PgVectorStore.java

+			.withNamespace(this.schemaName)
+			.withSimilarityMetric(this.getDistanceType().name())
+			.withIndexName(this.createIndexMethod.name())
+			.withModel(this.embeddingModel.getClass().getSimpleName());


This attribute should be the model name used for computing embedding (e.g. text-embedding-3-small). It's optional, so if we cannot get the value for now, I would leave this field empty. Also, since we have already instrumented the EmbeddingModel, we already have the model name in the trace as part of the embedding span, so we might even argue whether we want/need this here. It might be needed for cases where it's the vector database itself providing the embedding model, but as far as I know it's not something supported by the Spring AI APIs yet.

I left this open, until the Options PR is merged.
But even now it the EmbeddingModel does not expose getDefaultOptions method (similar to the ChatOptions) to show, at least, the default method name.
So we should either add the get options to the EmbeddingModel or drop this attribute all together.

I've decide to remove the Model attribute for now. We can consider adding it later if needed.

ThomasVitale · 2024-08-14T14:20:42Z

...spring-ai-pgvector-store/src/main/java/org/springframework/ai/vectorstore/PgVectorStore.java

+	@Override
+	public VectorStoreObservationContext.Builder createObservationContextBuilder(String operationName) {
+
+		return VectorStoreObservationContext.builder("pg_vector", operationName)


Consider introducing a VectorStoreProvider enum in the same package as AiProvider, and have the "pg_vector" value as part of that enum (but still leaving the field type as String, similar to how we handle the model provider in the LLM observations).

I will create an issue to consider this change once this PR is merged.

Added entry for the Simple Vector Store as well.

ThomasVitale · 2024-08-14T14:22:24Z

...ringframework/ai/autoconfigure/vectorstore/observation/VectorStoreObservationProperties.java

+@ConfigurationProperties(VectorStoreObservationProperties.CONFIG_PREFIX)
+public class VectorStoreObservationProperties {
+
+	public static final String CONFIG_PREFIX = "spring.ai.vector.store.observations";


The config properties for vector stores use the prefix spring.ai.vectorstore.*. Should this be using the same namespace?

Suggested change

public static final String CONFIG_PREFIX = "spring.ai.vector.store.observations";

public static final String CONFIG_PREFIX = "spring.ai.vectorstore.observations";

Agree, matching the extra 'dot' is not needed, it implies a sort of grouping under 'vector' as if there could be something else other than 'store'

- Introduce AbstractObservationVectorStore for instrumentation. Add observe instrumentation for the add, delete and the similaritySearch methods. - Add VectorStoreObservationContext for capturing operation details. - Implement DefaultVectorStoreObservationConvention for naming and tagging - Create filters for add and delete request content observation Implement VectorStoreQueryResponseContentObservationFilter. - Update VectorStore interface with getName() method, returning the class name by default. - Add VectorStoreObservationDocumentation for defining observation keys. - Create VectorStoreObservationAutoConfiguration for auto-configuring observations. - Add VectorStoreObservationProperties for configuring observation behavior. They are used to control the optional, additional, observationcontent filters. - Update PgVectorStoreAutoConfiguration to support observations. - Modify PgVectorStore to extend AbstractObservationVectorStore - Add observation support to PgVectorStore's Builder. Resolves spring-projects#1204

… QUERY values. Add tests for the VectorStore contex, convention and filters.

Add PgVectorObservationIT

ThomasVitale · 2024-08-16T07:16:33Z

Looks great!

tzolov added this to the 1.0.0-M2 milestone Aug 11, 2024

tzolov added vector store Observability labels Aug 11, 2024

tzolov changed the title ~~[WIP] Add observability support to VectorStore~~ Add observability support to VectorStore Aug 11, 2024

tzolov marked this pull request as ready for review August 11, 2024 08:30

tzolov force-pushed the vector-store-observation-support branch from 83e0413 to 75c5a9a Compare August 11, 2024 16:51

ThomasVitale reviewed Aug 14, 2024

View reviewed changes

tzolov force-pushed the vector-store-observation-support branch from 7804ffa to b58801b Compare August 15, 2024 13:37

tzolov added 8 commits August 16, 2024 08:38

Add VectorStoreObservationContext.Operation enum with ADD, DELETE and…

629e080

… QUERY values. Add tests for the VectorStore contex, convention and filters.

Add VectorStoreObservationAutoConfiguration tests

5dfbfb8

code cleaning

b005d5b

Add vector_store Spring AI kind

9196fea

Add observability support to the SimpleVectorStore.

5c71946

Add PgVectorObservationIT

Address most of the review comments

070f3a9

Remove the Add/Delete content observation support.

a0a71d2

tzolov force-pushed the vector-store-observation-support branch from b58801b to a0a71d2 Compare August 16, 2024 06:38

markpollack closed this in bc3f9ac Aug 16, 2024

tzolov mentioned this pull request Aug 19, 2024

Add observability support for the existing Vector Store implementations. #1243

Closed

17 tasks

	public static final String CONFIG_PREFIX = "spring.ai.vector.store.observations";
	public static final String CONFIG_PREFIX = "spring.ai.vectorstore.observations";

Add observability support to VectorStore #1205

Add observability support to VectorStore #1205

Uh oh!

Conversation

tzolov commented Aug 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasVitale Aug 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasVitale commented Aug 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ThomasVitale Aug 14, 2024 •

edited

Loading