Skip to content
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.

Sign up for GitHub

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

upgrade to SB 3.4 and polish observability #130

Merged
merged 4 commits into from
Dec 24, 2024
+177 −103
Merged

upgrade to SB 3.4 and polish observability #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d03f105
upgrade to SB 3.4 and polish observability
rajadilipkolli Dec 24, 2024
cbe0c01
upgrade docker
rajadilipkolli Dec 24, 2024
1cfa567
fix : tests
rajadilipkolli Dec 24, 2024
f1bc3a0
polish
rajadilipkolli Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 61 additions & 18 deletions rag/rag-springai-ollama-llm/docker/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,62 @@
services:
ollama:
image: langchain4j/ollama-llama3:latest
ports:
- '11434:11434'
redis-stack:
image: redis/redis-stack-server
ports:
- '6379:6379'
lgtm-stack:
image: grafana/otel-lgtm:0.8.0
extra_hosts: ['host.docker.internal:host-gateway']
container_name: lgtm-stack
environment:
- OTEL_METRIC_EXPORT_INTERVAL=500
ports:
- "3000:3000"
- "4317:4317"
- "4318:4318"

ollama:
container_name: ollama
image: ollama/ollama:latest
ports:
- '11434:11434'

postgresqldb:
container_name: postgresqldb
image: pgvector/pgvector:pg17
extra_hosts: [ 'host.docker.internal:host-gateway' ]
restart: always
environment:
- POSTGRES_USER=appuser
- POSTGRES_PASSWORD=secret
- POSTGRES_DB=appdb
- PGPASSWORD=secret
logging:
options:
max-size: 10m
max-file: "3"
ports:
- '5432:5432'
healthcheck:
test: "pg_isready -U appuser -d appdb"
interval: 2s
timeout: 20s
retries: 10

pgadmin:
container_name: pgadmin_container
image: dpage/pgadmin4
extra_hosts: [ 'host.docker.internal:host-gateway' ]
environment:
PGADMIN_DEFAULT_EMAIL: ${PGADMIN_DEFAULT_EMAIL:[email protected]}
PGADMIN_DEFAULT_PASSWORD: ${PGADMIN_DEFAULT_PASSWORD:-admin}
PGADMIN_CONFIG_SERVER_MODE: "False"
PGADMIN_CONFIG_MASTER_PASSWORD_REQUIRED: "False"
ports:
- "${PGADMIN_PORT:-5050}:80"
depends_on:
postgresqldb:
condition: service_healthy
volumes:
- ./docker_pgadmin_servers.json:/pgadmin4/servers.json
entrypoint:
- "/bin/sh"
- "-c"
- "/bin/echo 'postgresqldb:5432:*:appuser:secret' > /tmp/pgpassfile && chmod 600 /tmp/pgpassfile && /entrypoint.sh"

lgtm-stack:
image: grafana/otel-lgtm:0.8.1
extra_hosts: [ 'host.docker.internal:host-gateway' ]
container_name: lgtm-stack
environment:
- OTEL_METRIC_EXPORT_INTERVAL=500
ports:
- "3000:3000"
- "4317:4317"
- "4318:4318"
- "9090:9090"
14 changes: 14 additions & 0 deletions rag/rag-springai-ollama-llm/docker/docker_pgadmin_servers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"Servers": {
"1": {
"Name": "Docker Compose DB",
"Group": "Servers",
"Port": 5432,
"Username": "appuser",
"Host": "postgresqldb",
"SSLMode": "prefer",
"MaintenanceDB": "appdb",
"PassFile": "/tmp/pgpassfile"
}
}
}
48 changes: 34 additions & 14 deletions rag/rag-springai-ollama-llm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.6</version>
<version>3.4.1</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>com.learning.ai</groupId>
Expand All @@ -16,7 +16,8 @@

<properties>
<java.version>21</java.version>
<spring-ai.version>1.0.0-M4</spring-ai.version>
<spring-ai.version>1.0.0-M5</spring-ai.version>
<otelInstrumentation.version>2.10.0-alpha</otelInstrumentation.version>
<spotless.version>2.43.0</spotless.version>
</properties>

Expand All @@ -39,11 +40,7 @@
</dependency>
<dependency>
<groupId>org.springframework.ai</groupId>
<artifactId>spring-ai-redis-store-spring-boot-starter</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-pool2</artifactId>
<artifactId>spring-ai-pgvector-store-spring-boot-starter</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.ai</groupId>
Expand All @@ -56,7 +53,14 @@
<dependency>
<groupId>org.springdoc</groupId>
<artifactId>springdoc-openapi-starter-webmvc-ui</artifactId>
<version>2.6.0</version>
<version>2.7.0</version>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-devtools</artifactId>
<scope>runtime</scope>
<optional>true</optional>
</dependency>

<dependency>
Expand All @@ -76,6 +80,17 @@
<groupId>io.micrometer</groupId>
<artifactId>micrometer-registry-otlp</artifactId>
</dependency>
<dependency>
<groupId>net.ttddyy.observation</groupId>
<artifactId>datasource-micrometer-spring-boot</artifactId>
<version>1.0.6</version>
</dependency>
<dependency>
<groupId>io.opentelemetry.instrumentation</groupId>
<artifactId>opentelemetry-logback-appender-1.0</artifactId>
<scope>runtime</scope>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
Expand All @@ -97,16 +112,14 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.redis.testcontainers</groupId>
<artifactId>testcontainers-redis</artifactId>
<groupId>org.testcontainers</groupId>
<artifactId>grafana</artifactId>
<scope>test</scope>
<version>1.6.4</version>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>grafana</artifactId>
<artifactId>postgresql</artifactId>
<scope>test</scope>
<version>1.20.4</version>
</dependency>
<dependency>
<groupId>io.rest-assured</groupId>
Expand All @@ -124,6 +137,13 @@
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>io.opentelemetry.instrumentation</groupId>
<artifactId>opentelemetry-instrumentation-bom-alpha</artifactId>
<version>${otelInstrumentation.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

Expand All @@ -144,7 +164,7 @@
<configuration>
<java>
<palantirJavaFormat>
<version>2.47.0</version>
<version>2.50.0</version>
</palantirJavaFormat>
<importOrder/>
<removeUnusedImports/>
Expand Down
20 changes: 0 additions & 20 deletions ...-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java
View file
Open in desktop

This file was deleted.

43 changes: 19 additions & 24 deletions ...ai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.ai.chat.client.ChatClient;
import org.springframework.ai.chat.client.advisor.QuestionAnswerAdvisor;
import org.springframework.ai.chat.model.ChatResponse;
import org.springframework.ai.chat.model.Generation;
import org.springframework.ai.vectorstore.SearchRequest;
import org.springframework.ai.chat.client.advisor.RetrievalAugmentationAdvisor;
import org.springframework.ai.rag.generation.augmentation.ContextualQueryAugmenter;
import org.springframework.ai.rag.retrieval.search.VectorStoreDocumentRetriever;
import org.springframework.ai.vectorstore.VectorStore;
import org.springframework.stereotype.Service;

Expand All @@ -15,33 +14,29 @@ public class AIChatService {

private static final Logger LOGGER = LoggerFactory.getLogger(AIChatService.class);

private static final String template =
"""
You are a helpful assistant, conversing with a user about the subjects contained in a set of documents.
Use the information from the DOCUMENTS section to provide accurate answers. If unsure or if the answer
isn't found in the DOCUMENTS section, simply state that you don't know the answer.
private final ChatClient aiClient;

DOCUMENTS:
{question_answer_context}
public AIChatService(ChatClient.Builder builder, VectorStore vectorStore) {

""";
var documentRetriever = VectorStoreDocumentRetriever.builder()
.vectorStore(vectorStore)
.similarityThreshold(0.50)
.build();

private final ChatClient aiClient;
private final VectorStore vectorStore;
var queryAugmenter =
ContextualQueryAugmenter.builder().allowEmptyContext(true).build();

public AIChatService(ChatClient.Builder builder, VectorStore vectorStore) {
this.aiClient = builder.build();
this.vectorStore = vectorStore;
RetrievalAugmentationAdvisor retrievalAugmentationAdvisor = RetrievalAugmentationAdvisor.builder()
.documentRetriever(documentRetriever)
.queryAugmenter(queryAugmenter)
.build();
this.aiClient =
builder.clone().defaultAdvisors(retrievalAugmentationAdvisor).build();
}

public String chat(String query) {
ChatResponse aiResponse = aiClient.prompt()
.advisors(new QuestionAnswerAdvisor(vectorStore, SearchRequest.query(query), template))
.user(query)
.call()
.chatResponse();
String aiResponse = aiClient.prompt().user(query).call().content();
LOGGER.info("Response received from call :{}", aiResponse);
Generation generation = aiResponse.getResult();
return (generation != null) ? generation.getOutput().getContent() : "";
return aiResponse;
Comment on lines 37 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and observability.

The current implementation lacks proper error handling and could benefit from enhanced observability, especially given the PR's objective to "polish observability".

Consider implementing these improvements:

 public String chat(String query) {
+    if (query == null || query.trim().isEmpty()) {
+        throw new IllegalArgumentException("Query cannot be empty");
+    }
+    LOGGER.debug("Processing chat query: {}", query);
+    try {
+        long startTime = System.currentTimeMillis();
         String aiResponse = aiClient.prompt().user(query).call().content();
-        LOGGER.info("Response received from call :{}", aiResponse);
+        long duration = System.currentTimeMillis() - startTime;
+        LOGGER.info("Chat response received in {}ms. Query: '{}', Response: '{}'", 
+            duration, query, aiResponse);
         return aiResponse;
+    } catch (Exception e) {
+        LOGGER.error("Error processing chat query: '{}'. Error: {}", 
+            query, e.getMessage(), e);
+        throw new RuntimeException("Failed to process chat query", e);
+    }
 }
📝 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.

Suggested change
public String chat(String query) {
ChatResponse aiResponse = aiClient.prompt()
.advisors(new QuestionAnswerAdvisor(vectorStore, SearchRequest.query(query), template))
.user(query)
.call()
.chatResponse();
String aiResponse = aiClient.prompt().user(query).call().content();
LOGGER.info("Response received from call :{}", aiResponse);
Generation generation = aiResponse.getResult();
return (generation != null) ? generation.getOutput().getContent() : "";
return aiResponse;
public String chat(String query) {
if (query == null || query.trim().isEmpty()) {
throw new IllegalArgumentException("Query cannot be empty");
}
LOGGER.debug("Processing chat query: {}", query);
try {
long startTime = System.currentTimeMillis();
String aiResponse = aiClient.prompt().user(query).call().content();
long duration = System.currentTimeMillis() - startTime;
LOGGER.info("Chat response received in {}ms. Query: '{}', Response: '{}'",
duration, query, aiResponse);
return aiResponse;
} catch (Exception e) {
LOGGER.error("Error processing chat query: '{}'. Error: {}",
query, e.getMessage(), e);
throw new RuntimeException("Failed to process chat query", e);
}
}

}
}
5 changes: 3 additions & 2 deletions ...lama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.learning.ai.llmragwithspringai.service;

import java.util.Map;
import java.util.Objects;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.ai.document.DocumentReader;
Expand Down Expand Up @@ -58,11 +59,11 @@ public void loadData(Resource documentResource) {
return documents;
};
vectorStore.accept(metadataEnricher.apply(tokenTextSplitter.apply(documentReader.get())));
LOGGER.info("Loaded document to redis vector database.");
LOGGER.info("Loaded document to vector database.");
}
}

public long count() {
return this.vectorStore.similaritySearch("*").size();
return Objects.requireNonNull(this.vectorStore.similaritySearch("*")).size();
}
}
11 changes: 11 additions & 0 deletions rag/rag-springai-ollama-llm/src/main/resources/application-local.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## WARNING: Development-only configuration
## The following setting will DELETE existing vector store data on startup
spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Data deletion setting found in default configuration

The dangerous vector store table removal setting is not properly isolated. It's currently set in:

  • rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (local profile)
  • embeddingstores/pgvector-springai/src/main/resources/application.properties (default configuration)

Having this setting in the default application.properties is risky as it could lead to unintended data loss in production. This setting should only exist in local development profiles.

Recommendations:

  • Remove spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true from embeddingstores/pgvector-springai/src/main/resources/application.properties
  • Keep it only in *-local.properties files
  • Consider adding a warning comment in the default properties file about this setting being development-only
🔗 Analysis chain

Verify configuration isolation for data deletion setting

The vector store table removal setting is dangerous and should be strictly isolated to local development.

Let's verify this setting isn't present in other profiles:

Let me verify if this setting is properly isolated by checking the context of its usage in the other file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for vector store removal setting in all property files
fd -e properties -e yml -e yaml | xargs rg "spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable"

Length of output: 392

Script:

#!/bin/bash
# Get context for the other occurrence
rg -B2 -A2 "spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable" embeddingstores/pgvector-springai/src/main/resources/application.properties

Length of output: 342

spring.ai.ollama.baseUrl=http://localhost:11434


logging.level.org.springframework.ai.rag=debug

spring.datasource.url=jdbc:postgresql://localhost/appdb
spring.datasource.username=appuser
spring.datasource.password=secret
Comment on lines +9 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Secure database credentials and add connection pool settings

Several security and configuration concerns:

  1. Database credentials should not be stored in properties files. Use environment variables or a secure configuration management system.
  2. Connection pool settings are missing, which could affect application performance.

Apply these changes:

-spring.datasource.url=jdbc:postgresql://localhost/appdb
-spring.datasource.username=appuser
-spring.datasource.password=secret
+spring.datasource.url=${SPRING_DATASOURCE_URL:jdbc:postgresql://localhost/appdb}
+spring.datasource.username=${SPRING_DATASOURCE_USERNAME}
+spring.datasource.password=${SPRING_DATASOURCE_PASSWORD}
+
+# Connection pool settings
+spring.datasource.hikari.maximum-pool-size=10
+spring.datasource.hikari.minimum-idle=5
+spring.datasource.hikari.idle-timeout=300000

Also, consider using Spring Cloud Config Server or HashiCorp Vault for secure credential management in a production environment.

📝 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.

Suggested change
spring.datasource.url=jdbc:postgresql://localhost/appdb
spring.datasource.username=appuser
spring.datasource.password=secret
spring.datasource.url=${SPRING_DATASOURCE_URL:jdbc:postgresql://localhost/appdb}
spring.datasource.username=${SPRING_DATASOURCE_USERNAME}
spring.datasource.password=${SPRING_DATASOURCE_PASSWORD}
# Connection pool settings
spring.datasource.hikari.maximum-pool-size=10
spring.datasource.hikari.minimum-idle=5
spring.datasource.hikari.idle-timeout=300000

14 changes: 10 additions & 4 deletions rag/rag-springai-ollama-llm/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,32 @@ spring.application.name=rag-springai-ollama-llm
spring.threads.virtual.enabled=true
spring.mvc.problemdetails.enabled=true

spring.ai.ollama.init.pull-model-strategy=WHEN_MISSING
spring.ai.ollama.chat.options.model=mistral
spring.ai.ollama.chat.options.temperature=0.3
spring.ai.ollama.chat.options.top-k=2
spring.ai.ollama.chat.options.top-p=0.2

spring.ai.ollama.embedding.options.model=nomic-embed-text

spring.ai.vectorstore.redis.index=vector_store
spring.ai.vectorstore.redis.prefix=ai
spring.ai.vectorstore.redis.initialize-schema=true
#PgVector
spring.ai.vectorstore.observations.include-query-response=true
spring.ai.vectorstore.pgvector.initialize-schema=true
Comment on lines +14 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Missing required PostgreSQL connection properties for PgVector

The PgVector configuration is incomplete. Based on the example from pgvector-springai, the following essential properties are missing:

  • spring.datasource.url
  • spring.datasource.username
  • spring.datasource.password

These properties should be added to either application.properties or application-local.properties (which currently doesn't exist) to ensure proper connection to the PostgreSQL database with PgVector extension.

Example of required properties:

spring.datasource.url=jdbc:postgresql://localhost/appdb
spring.datasource.username=appuser
spring.datasource.password=secret
🔗 Analysis chain

Verify PgVector connection properties

While schema initialization is configured, the PostgreSQL connection properties appear to be missing. These are typically required for PgVector to function.

Let's check if these properties are defined elsewhere:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for PostgreSQL connection properties in all property files
fd -e properties -e yaml | xargs rg -l "spring\.datasource\.(url|username|password)"

Length of output: 403

Script:

#!/bin/bash
# Let's check the content of application-local.properties in the same directory
cat rag/rag-springai-ollama-llm/src/main/resources/application-local.properties 2>/dev/null || echo "File not found"

# Also check if there are any other property files in the same directory
ls rag/rag-springai-ollama-llm/src/main/resources/

# Let's also examine one of the found PgVector examples to see the required properties
cat embeddingstores/pgvector-springai/src/main/resources/application-local.properties

Length of output: 428


spring.ai.ollama.baseUrl=http://localhost:11434
spring.http.client.connect-timeout=PT1M
spring.http.client.read-timeout=PT5M

spring.testcontainers.beans.startup=parallel

##Observability
spring.ai.chat.observations.include-completion=true
spring.ai.chat.observations.include-prompt=true
spring.ai.chat.client.observations.include-input=true

management.endpoints.web.exposure.include=*
management.metrics.tags.service.name=${spring.application.name}
management.tracing.sampling.probability=1.0
management.otlp.tracing.endpoint=http://localhost:4318/v1/traces
management.otlp.logging.endpoint=http://localhost:4318/v1/logs
Comment on lines 28 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security: Restrict management endpoint exposure

Exposing all management endpoints (management.endpoints.web.exposure.include=*) could pose a security risk in production environments.

Consider:

  1. Limiting exposed endpoints to necessary ones only
  2. Securing the endpoints with appropriate authentication
-management.endpoints.web.exposure.include=*
+management.endpoints.web.exposure.include=health,metrics,prometheus,info
+management.endpoints.web.exposure.exclude=env,beans
📝 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.

Suggested change
management.endpoints.web.exposure.include=*
management.metrics.tags.service.name=${spring.application.name}
management.tracing.sampling.probability=1.0
management.otlp.tracing.endpoint=http://localhost:4318/v1/traces
management.otlp.logging.endpoint=http://localhost:4318/v1/logs
management.endpoints.web.exposure.include=health,metrics,prometheus,info
management.endpoints.web.exposure.exclude=env,beans
management.metrics.tags.service.name=${spring.application.name}
management.tracing.sampling.probability=1.0
management.otlp.tracing.endpoint=http://localhost:4318/v1/traces
management.otlp.logging.endpoint=http://localhost:4318/v1/logs


logging.level.org.springframework.ai.rag=info
16 changes: 16 additions & 0 deletions rag/rag-springai-ollama-llm/src/main/resources/logback-spring.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration>
<include resource="org/springframework/boot/logging/logback/base.xml" />

<appender name="OpenTelemetry"
class="io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppender">
<captureExperimentalAttributes>true</captureExperimentalAttributes>
<captureCodeAttributes>true</captureCodeAttributes>
<captureKeyValuePairAttributes>true</captureKeyValuePairAttributes>
</appender>

<root level="INFO">
<appender-ref ref="OpenTelemetry"/>
</root>

</configuration>
Loading
Loading