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.

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

[OPIK-595]: Add Redis Caching part 1 #1015

Merged
merged 13 commits into from
Jan 14, 2025
13 changes: 13 additions & 0 deletions apps/opik-backend/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,16 @@ llmProviderClient:
# Default: 2023-06-01
# Description: Anthropic API version https://docs.anthropic.com/en/api/versioning
version: ${LLM_PROVIDER_ANTHROPIC_VERSION:-'2023-06-01'}

# Configuration for cache manager
cacheManager:
# Default: true
# Description: Whether or not cache manager is enabled
enabled: ${CACHE_MANAGER_ENABLED:-true}
# Default: PT1S
# Description: Time to live for cache entries in using the formats accepted are based on the ISO-8601: https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html#parse-java.lang.CharSequence-
defaultDuration: ${CACHE_MANAGER_DEFAULT_DURATION:-PT1S}
caches:
# Default: {}
# Description: Dynamically created caches with their respective time to live in seconds
automationRules: ${CACHE_MANAGER_AUTOMATION_RULES_DURATION:-PT1S}
10 changes: 9 additions & 1 deletion apps/opik-backend/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@
<artifactId>httpclient5</artifactId>
<version>5.2.3</version>
</dependency>
<dependency>
<groupId>org.mvel</groupId>
<artifactId>mvel2</artifactId>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: we could evaluate using Drools as well. But this is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you believe Drools, it's better that we use it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be ok to go with this as well.

<version>2.5.2.Final</version>
</dependency>
<dependency>
<groupId>org.mapstruct</groupId>
<artifactId>mapstruct</artifactId>
Expand Down Expand Up @@ -290,7 +295,7 @@
<dependency>
<groupId>com.clickhouse</groupId>
<artifactId>clickhouse-jdbc</artifactId>
<version>0.7.0</version>
<version>${clickhouse-java.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -446,6 +451,9 @@
<configuration>
<source>21</source>
<target>21</target>
<compilerArgument>-parameters</compilerArgument>
<parameters>true</parameters>
<testCompilerArgument>-parameters</testCompilerArgument>
<annotationProcessorPaths>
<path>
<groupId>org.projectlombok</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.comet.opik.infrastructure.bi.BiModule;
import com.comet.opik.infrastructure.bi.OpikGuiceyLifecycleEventListener;
import com.comet.opik.infrastructure.bundle.LiquibaseBundle;
import com.comet.opik.infrastructure.cache.CacheModule;
import com.comet.opik.infrastructure.db.DatabaseAnalyticsModule;
import com.comet.opik.infrastructure.db.IdGeneratorModule;
import com.comet.opik.infrastructure.db.NameGeneratorModule;
Expand Down Expand Up @@ -73,7 +74,7 @@ public void initialize(Bootstrap<OpikConfiguration> bootstrap) {
.withPlugins(new SqlObjectPlugin(), new Jackson2Plugin()))
.modules(new DatabaseAnalyticsModule(), new IdGeneratorModule(), new AuthModule(), new RedisModule(),
new RateLimitModule(), new NameGeneratorModule(), new HttpModule(), new EventModule(),
new ConfigurationModule(), new BiModule(), new LlmProviderClientModule())
new ConfigurationModule(), new BiModule(), new CacheModule(), new LlmProviderClientModule())
.installers(JobGuiceyInstaller.class)
.listen(new OpikGuiceyLifecycleEventListener())
.enableAutoConfig()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public sealed interface AutomationRuleEvaluatorModel<T> extends AutomationRuleMo

@Json
T code();

AutomationRuleEvaluatorType type();

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ List<AutomationRuleEvaluatorLlmAsJudge> findAll(@NonNull UUID projectId, @NonNul
AutomationRuleEvaluatorType automationRuleEvaluatorType);
}

@NonNull @Singleton
@Singleton
@RequiredArgsConstructor(onConstructor_ = @Inject)
@Slf4j
class AutomationRuleEvaluatorServiceImpl implements AutomationRuleEvaluatorService {
Expand All @@ -61,7 +61,7 @@ class AutomationRuleEvaluatorServiceImpl implements AutomationRuleEvaluatorServi
private final @NonNull TransactionTemplate template;

@Override
public <E, T extends AutomationRuleEvaluator<E>> T save(T inputRuleEvaluator,
public <E, T extends AutomationRuleEvaluator<E>> T save(@NonNull T inputRuleEvaluator,
@NonNull UUID projectId,
@NonNull String workspaceId,
@NonNull String userName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,12 @@ class LlmProviderApiKeyServiceImpl implements LlmProviderApiKeyService {
@Override
public ProviderApiKey find(@NonNull UUID id, @NonNull String workspaceId) {

ProviderApiKey providerApiKey = template.inTransaction(READ_ONLY, handle -> {
return template.inTransaction(READ_ONLY, handle -> {

var repository = handle.attach(LlmProviderApiKeyDAO.class);

return repository.fetch(id, workspaceId).orElseThrow(this::createNotFoundError);
});

return providerApiKey;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.comet.opik.infrastructure;

import com.fasterxml.jackson.annotation.JsonProperty;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import lombok.Data;

import java.time.Duration;
import java.util.Map;
import java.util.Optional;

@Data
public class CacheConfiguration {

@Valid @JsonProperty
private boolean enabled = false;

@Valid @JsonProperty
@NotNull private Duration defaultDuration;

@Valid @JsonProperty
private Map<String, Duration> caches;

public Map<String, Duration> getCaches() {
return Optional.ofNullable(caches).orElse(Map.of());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,7 @@ public class OpikConfiguration extends JobConfiguration {

@Valid @NotNull @JsonProperty
private LlmProviderClientConfig llmProviderClient = new LlmProviderClientConfig();

@Valid @NotNull @JsonProperty
private CacheConfiguration cacheManager = new CacheConfiguration();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.comet.opik.infrastructure.cache;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target({ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@Documented
public @interface CacheEvict {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We discussed the JCache approach, but what about just reusing their interface annotations. Any blocker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JCache annotations only have a cache name property and other configuration classes, such as a cache resolver and a key generator (which must be implemented using reflection based on the method signature). For those reasons, I kept these annotations, which have a cache name and key. We use MVEL to solve the key expression. I believe this is less verbose and a bit more flexible.


/**
* @return the name of the cache group.
* */
String name();

/**
* key is a SpEL expression implemented using MVEL. Please refer to the <a href="http://mvel.documentnode.com/">MVEL documentation for more information</a>.
*
* @return SpEL expression evaluated to generate the cache key.
* */
String key();
}
Loading
Loading