From f27f88a2f9489ad4c6cc186402030499073041de Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Mon, 6 Jul 2026 19:06:23 +0200 Subject: [PATCH 1/4] [OPIK-7256] fix(backend): make project size-estimate cache actually engage The @Cacheable on getProjectMetadata never fired: Guice method interception cannot intercept private methods, and the cache name project_metadata was absent from cacheManager.caches (only an orphaned workspace_metadata entry existed), so even an intercepted call would have fallen back to the 1-second default TTL. The size-estimate query therefore ran on essentially every traces/spans find request. - make the method package-private so interception applies - rename the orphaned workspace_metadata config entry to project_metadata (env CACHE_MANAGER_PROJECT_METADATA_DURATION, PT1H) Co-Authored-By: Claude Fable 5 --- apps/opik-backend/config.yml | 6 +++++- .../opik/domain/workspaces/WorkspaceMetadataService.java | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/opik-backend/config.yml b/apps/opik-backend/config.yml index d6789d81728..e3b85bbf14d 100644 --- a/apps/opik-backend/config.yml +++ b/apps/opik-backend/config.yml @@ -1021,7 +1021,11 @@ cacheManager: # Default: {} # Description: Dynamically created caches with their respective time to live in seconds automationRules: ${CACHE_MANAGER_AUTOMATION_RULES_DURATION:-PT1S} - workspace_metadata: ${CACHE_MANAGER_WORKSPACE_METADATA_DURATION:-PT1H} + # Default: PT1H + # Description: Cache TTL for the per-project size estimate (ScopeMetadata) gating dynamic sorting. + # The estimate scans all of the project's spans, so cache misses are expensive by design. + # Renamed from the orphaned workspace_metadata entry, which no code referenced. + project_metadata: ${CACHE_MANAGER_PROJECT_METADATA_DURATION:-PT1H} # Default: PT5M # Description: Cache TTL for workspace version determination results, applied per workspace # Scope: Per workspace (cache key includes workspace ID) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java index 91112da5a7c..73e761a6a9e 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java @@ -44,8 +44,10 @@ public Mono getProjectMetadata(@NonNull String workspaceId, UUID .flatMap(resolvedProjectId -> getProjectMetadata(workspaceId, resolvedProjectId)); } + // Package-private, not private: Guice method interception (which implements @Cacheable) cannot + // intercept private methods, so a private modifier silently disables the cache. @Cacheable(name = "project_metadata", key = "'-'+ $workspaceId + '-' + $projectId", returnType = ScopeMetadata.class) - private Mono getProjectMetadata(String workspaceId, UUID projectId) { + Mono getProjectMetadata(String workspaceId, UUID projectId) { return workspaceMetadataDAO.getProjectMetadata(workspaceId, projectId); } From 819bfa8f3f80fb40ed2d033de4fe1686485e180f Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Mon, 6 Jul 2026 19:44:24 +0200 Subject: [PATCH 2/4] [OPIK-7256] test: guard that the project_metadata cache engages Counting-stub DAO behind the real service in the app harness: a second call with the same key must not reach the DAO. Fails if the method becomes non-interceptable again (e.g. private) or the cache name is dropped from configuration. Co-Authored-By: Claude Fable 5 --- .../WorkspaceMetadataServiceCacheTest.java | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java diff --git a/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java b/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java new file mode 100644 index 00000000000..401bf2e318e --- /dev/null +++ b/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java @@ -0,0 +1,129 @@ +package com.comet.opik.domain.workspaces; + +import com.comet.opik.api.resources.utils.ClickHouseContainerUtils; +import com.comet.opik.api.resources.utils.MigrationUtils; +import com.comet.opik.api.resources.utils.MySQLContainerUtils; +import com.comet.opik.api.resources.utils.RedisContainerUtils; +import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils; +import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils.AppContextConfig; +import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils.CustomConfig; +import com.comet.opik.extensions.DropwizardAppExtensionProvider; +import com.comet.opik.extensions.RegisterApp; +import com.google.inject.AbstractModule; +import com.redis.testcontainers.RedisContainer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.testcontainers.clickhouse.ClickHouseContainer; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.lifecycle.Startables; +import org.testcontainers.mysql.MySQLContainer; +import reactor.core.publisher.Mono; +import ru.vyarus.dropwizard.guice.test.jupiter.ext.TestDropwizardAppExtension; + +import java.util.List; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicInteger; + +import static com.comet.opik.api.resources.utils.ClickHouseContainerUtils.DATABASE_NAME; +import static com.comet.opik.api.resources.utils.TestUtils.waitForMillis; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards that the {@code project_metadata} cache on + * {@link WorkspaceMetadataServiceImpl#getProjectMetadata(String, UUID)} actually engages: the + * method must stay interceptable by Guice (not private) and the cache name must stay configured, + * otherwise the expensive per-project size estimate silently runs on every call. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@ExtendWith(DropwizardAppExtensionProvider.class) +class WorkspaceMetadataServiceCacheTest { + + private final RedisContainer REDIS = RedisContainerUtils.newRedisContainer(); + private final MySQLContainer MYSQL = MySQLContainerUtils.newMySQLContainer(); + private final GenericContainer ZOOKEEPER_CONTAINER = ClickHouseContainerUtils.newZookeeperContainer(); + private final ClickHouseContainer CLICKHOUSE = ClickHouseContainerUtils.newClickHouseContainer(ZOOKEEPER_CONTAINER); + + private static final long CACHE_ASYNC_WAIT_MILLIS = 100; + + private static final AtomicInteger PROJECT_METADATA_DAO_CALLS = new AtomicInteger(); + + private static final WorkspaceMetadataDAO COUNTING_DAO = new WorkspaceMetadataDAO() { + + @Override + public Mono getProjectMetadata(String workspaceId, UUID projectId) { + PROJECT_METADATA_DAO_CALLS.incrementAndGet(); + return Mono.just(ScopeMetadata.builder() + .sizeGb(ThreadLocalRandom.current().nextDouble()) + .totalTableSizeGb(100) + .percentageOfTable(1) + .limitSizeGb(10) + .build()); + } + + @Override + public Mono getExperimentMetadata(String workspaceId, UUID datasetId) { + return Mono.just(ExperimentScopeMetadata.builder().build()); + } + }; + + @RegisterApp + private final TestDropwizardAppExtension APP; + + { + Startables.deepStart(MYSQL, CLICKHOUSE, REDIS, ZOOKEEPER_CONTAINER).join(); + + var databaseAnalyticsFactory = ClickHouseContainerUtils.newDatabaseAnalyticsFactory( + CLICKHOUSE, DATABASE_NAME); + + MigrationUtils.runMysqlDbMigration(MYSQL); + MigrationUtils.runClickhouseDbMigration(CLICKHOUSE); + + APP = TestDropwizardAppExtensionUtils.newTestDropwizardAppExtension( + AppContextConfig.builder() + .jdbcUrl(MYSQL.getJdbcUrl()) + .databaseAnalyticsFactory(databaseAnalyticsFactory) + .redisUrl(REDIS.getRedisURI()) + .modules(List.of(new AbstractModule() { + + @Override + protected void configure() { + bind(WorkspaceMetadataDAO.class).toInstance(COUNTING_DAO); + } + + })) + .customConfigs( + List.of( + new CustomConfig("cacheManager.enabled", "true"), + new CustomConfig("cacheManager.caches.project_metadata", "PT30S"))) + .build()); + } + + @Test + void getProjectMetadata__repeatedCallsAreServedFromCache(WorkspaceMetadataService service) { + var impl = (WorkspaceMetadataServiceImpl) service; + var workspaceId = UUID.randomUUID().toString(); + var projectId = UUID.randomUUID(); + + var first = impl.getProjectMetadata(workspaceId, projectId).block(); + + assertThat(first).isNotNull(); + assertThat(PROJECT_METADATA_DAO_CALLS.get()).isEqualTo(1); + + // wait for the cache put async to complete + waitForMillis(CACHE_ASYNC_WAIT_MILLIS); + + // same key: served from cache, the DAO must not be called again + var second = impl.getProjectMetadata(workspaceId, projectId).block(); + + assertThat(second).isEqualTo(first); + assertThat(PROJECT_METADATA_DAO_CALLS.get()).isEqualTo(1); + + // different project: cache miss, the DAO is called again + var other = impl.getProjectMetadata(workspaceId, UUID.randomUUID()).block(); + + assertThat(other).isNotEqualTo(first); + assertThat(PROJECT_METADATA_DAO_CALLS.get()).isEqualTo(2); + } +} From 7ebbfd2bf55444726830eccef6baac268dfda568 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Mon, 6 Jul 2026 19:58:20 +0200 Subject: [PATCH 3/4] [OPIK-7256] test: replace fixed sleep with deterministic wait on the cache entry The interceptor drops the putAsync completion signal, so poll CacheManager.contains with a bounded timeout instead of sleeping. Co-Authored-By: Claude Fable 5 --- .../WorkspaceMetadataServiceCacheTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java b/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java index 401bf2e318e..c2db224cc44 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/domain/workspaces/WorkspaceMetadataServiceCacheTest.java @@ -9,8 +9,10 @@ import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils.CustomConfig; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; +import com.comet.opik.infrastructure.cache.CacheManager; import com.google.inject.AbstractModule; import com.redis.testcontainers.RedisContainer; +import org.awaitility.Awaitility; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; @@ -21,13 +23,13 @@ import reactor.core.publisher.Mono; import ru.vyarus.dropwizard.guice.test.jupiter.ext.TestDropwizardAppExtension; +import java.time.Duration; import java.util.List; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicInteger; import static com.comet.opik.api.resources.utils.ClickHouseContainerUtils.DATABASE_NAME; -import static com.comet.opik.api.resources.utils.TestUtils.waitForMillis; import static org.assertj.core.api.Assertions.assertThat; /** @@ -45,8 +47,6 @@ class WorkspaceMetadataServiceCacheTest { private final GenericContainer ZOOKEEPER_CONTAINER = ClickHouseContainerUtils.newZookeeperContainer(); private final ClickHouseContainer CLICKHOUSE = ClickHouseContainerUtils.newClickHouseContainer(ZOOKEEPER_CONTAINER); - private static final long CACHE_ASYNC_WAIT_MILLIS = 100; - private static final AtomicInteger PROJECT_METADATA_DAO_CALLS = new AtomicInteger(); private static final WorkspaceMetadataDAO COUNTING_DAO = new WorkspaceMetadataDAO() { @@ -101,7 +101,8 @@ protected void configure() { } @Test - void getProjectMetadata__repeatedCallsAreServedFromCache(WorkspaceMetadataService service) { + void getProjectMetadata__repeatedCallsAreServedFromCache(WorkspaceMetadataService service, + CacheManager cacheManager) { var impl = (WorkspaceMetadataServiceImpl) service; var workspaceId = UUID.randomUUID().toString(); var projectId = UUID.randomUUID(); @@ -111,8 +112,12 @@ void getProjectMetadata__repeatedCallsAreServedFromCache(WorkspaceMetadataServic assertThat(first).isNotNull(); assertThat(PROJECT_METADATA_DAO_CALLS.get()).isEqualTo(1); - // wait for the cache put async to complete - waitForMillis(CACHE_ASYNC_WAIT_MILLIS); + // the cache put is async and its completion signal is dropped, so wait until the entry is + // actually readable before asserting the second call is served from the cache + var cacheKey = "project_metadata:--%s-%s".formatted(workspaceId, projectId); + Awaitility.await() + .atMost(Duration.ofSeconds(5)) + .until(() -> Boolean.TRUE.equals(cacheManager.contains(cacheKey).block())); // same key: served from cache, the DAO must not be called again var second = impl.getProjectMetadata(workspaceId, projectId).block(); From acd802b12fc984a531df4ef5e4da5df7e4ac38f4 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Mon, 6 Jul 2026 23:36:35 +0200 Subject: [PATCH 4/4] [OPIK-7256] make the cached overload public, matching codebase convention All other @Cacheable methods are public; the class stays package-private so this widens nothing, and the comment keeps the not-private constraint. Co-Authored-By: Claude Fable 5 --- .../opik/domain/workspaces/WorkspaceMetadataService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java index 73e761a6a9e..0df3f138c11 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/workspaces/WorkspaceMetadataService.java @@ -44,10 +44,10 @@ public Mono getProjectMetadata(@NonNull String workspaceId, UUID .flatMap(resolvedProjectId -> getProjectMetadata(workspaceId, resolvedProjectId)); } - // Package-private, not private: Guice method interception (which implements @Cacheable) cannot + // Must not be private: Guice method interception (which implements @Cacheable) cannot // intercept private methods, so a private modifier silently disables the cache. @Cacheable(name = "project_metadata", key = "'-'+ $workspaceId + '-' + $projectId", returnType = ScopeMetadata.class) - Mono getProjectMetadata(String workspaceId, UUID projectId) { + public Mono getProjectMetadata(String workspaceId, UUID projectId) { return workspaceMetadataDAO.getProjectMetadata(workspaceId, projectId); }