Skip to content

Commit dd22f51

Browse files
authored
Merge branch 'main' into yaricom/OPIK-5555-ollie-trace-context-fix
2 parents 2daff24 + e056c1b commit dd22f51

93 files changed

Lines changed: 4028 additions & 618 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/sync_provider_models.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,34 @@ jobs:
8585
git add apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/
8686
git add apps/opik-frontend/src/types/providers.ts
8787
git add apps/opik-frontend/src/hooks/useLLMProviderModelsData.ts
88+
git add apps/opik-backend/src/main/resources/llm-models-default.yaml
8889
git commit -m "[NA] [BE][FE] chore: sync provider model definitions"
8990
91+
- name: Configure AWS credentials
92+
if: steps.sync.outputs.exit_code == '0'
93+
uses: aws-actions/configure-aws-credentials@v4
94+
with:
95+
aws-access-key-id: ${{ secrets.AWS_OPIK_CDN_KEY_ID }}
96+
aws-secret-access-key: ${{ secrets.AWS_OPIK_CDN_SECRERT }}
97+
aws-region: us-east-1
98+
role-to-assume: ${{ vars.AWS_OPIK_CDN_ROLE }}
99+
role-session-name: sync-provider-models
100+
101+
- name: Upload YAML to S3
102+
if: steps.sync.outputs.exit_code == '0'
103+
run: |
104+
aws s3 cp \
105+
apps/opik-backend/src/main/resources/llm-models-default.yaml \
106+
${{ vars.AWS_OPIK_CDN_BUCKET }}/llm-models-default.yaml \
107+
--cache-control "max-age=300"
108+
109+
- name: Invalidate CloudFront distribution
110+
if: steps.sync.outputs.exit_code == '0'
111+
run: |
112+
aws cloudfront create-invalidation \
113+
--distribution-id ${{ secrets.AWS_OPIK_CDN_DISTRIBUTION }} \
114+
--paths "/opik/llm-models-default.yaml"
115+
90116
- name: Create Pull Request
91117
if: steps.check_changes.outputs.has_changes == 'true'
92118
uses: peter-evans/create-pull-request@v6

apps/opik-backend/config.yml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -595,9 +595,6 @@ onlineScoring:
595595

596596
# Configuration for Evaluation Suite assertions
597597
evalSuite:
598-
# Default: gpt-5-nano
599-
# Description: Default LLM model used for eval suite assertions when the evaluator config has no model specified
600-
defaultModelName: ${EVAL_SUITE_DEFAULT_MODEL_NAME:-gpt-5-nano}
601598
# Default: 1
602599
# Description: Number of LLM runs per dataset item during eval suite execution
603600
defaultRunsPerItem: ${EVAL_SUITE_DEFAULT_RUNS_PER_ITEM:-1}
@@ -967,10 +964,19 @@ serviceToggles:
967964
# Default: false
968965
# Description: Whether or not Collaborators tab feature is enabled
969966
collaboratorsTabEnabled: ${TOGGLE_COLLABORATORS_TAB_ENABLED:-"false"}
967+
# Default: "" (empty, no allowlisted workspaces)
968+
# Description: Comma-separated list of workspace IDs that should always receive V2 navigation.
969+
# Valid values: comma-separated workspace IDs (e.g. "ws-id-1,ws-id-2"), or "" (empty, disabled).
970+
# Scope: Per-workspace — only listed workspace IDs are affected, all others follow normal determination.
971+
# Priority: Highest — overrides forceWorkspaceVersion and all other determination logic for listed workspaces.
972+
# Use case: gradual V2 rollout — set forceWorkspaceVersion to "version_1" and allowlist specific workspace IDs for V2.
973+
# Operational impact: empty/missing means no allowlisting. Non-allowlisted workspaces are unaffected.
974+
v2WorkspaceAllowlist: ${TOGGLE_V2_WORKSPACE_ALLOWLIST:-""}
970975
# Default: version_1
971-
# Description: Forces ALL workspaces to a specific navigation version. Highest priority override.
976+
# Description: Forces ALL workspaces to a specific navigation version.
972977
# Valid values: "disabled" (use version 1 entity check), "version_1" (force legacy), "version_2" (force project-first).
973978
# Scope: Global — applies to all workspaces regardless of their entity state.
979+
# Priority: Second highest — overridden by v2WorkspaceAllowlist for listed workspaces.
974980
# Overrides: When not "disabled", overrides the version entity check and auth gate entirely.
975981
# Operational impact: Set to "version_1" until V2 is ready, then change to "disabled" to enable entity-based determination.
976982
forceWorkspaceVersion: ${TOGGLE_FORCE_WORKSPACE_VERSION:-"version_1"}

apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/BaseRedisSubscriber.java

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,11 @@ private void removeConsumer() {
285285
try {
286286
stream.removeConsumer(config.getConsumerGroupName(), consumerId)
287287
.subscribeOn(consumerScheduler)
288-
.doOnSuccess(pendingMessages -> log.info(
289-
"Removed consumer '{}', from group '{}', pendingMessages '{}'",
290-
consumerId, config.getConsumerGroupName(), pendingMessages))
288+
.doOnSuccess(pendingMessages -> {
289+
pendingMessages = Objects.requireNonNullElse(pendingMessages, 0L);
290+
log.info("Removed consumer '{}', from group '{}', pendingMessages '{}'",
291+
consumerId, config.getConsumerGroupName(), pendingMessages);
292+
})
291293
.onErrorResume(throwable -> {
292294
log.warn("Failed to remove consumer '{}', group '{}'",
293295
consumerId, config.getConsumerGroupName(), throwable);
@@ -378,6 +380,7 @@ private Mono<Map<StreamMessageId, Map<String, M>>> claimPendingMessages() {
378380
.map(AutoClaimResult::getMessages)
379381
.filter(Objects::nonNull)
380382
.doOnSuccess(claimedMessages -> {
383+
claimedMessages = Objects.requireNonNullElse(claimedMessages, Map.of());
381384
claimSize.set(claimedMessages.size());
382385
log.debug("Successfully auto claimed from stream, size '{}'", claimedMessages.size());
383386
})
@@ -400,7 +403,10 @@ private Mono<Map<StreamMessageId, Map<String, M>>> readMessages() {
400403
return stream.readGroup(config.getConsumerGroupName(), consumerId, streamReadGroupArgs)
401404
.subscribeOn(consumerScheduler) // Isolates the Redis call
402405
.filter(Objects::nonNull)
406+
// doOnSuccess fires with null for empty Monos (e.g. long-poll timeout).
407+
// Defaulting to empty map to avoid NullPointerException, and for the gauge to reset to 0
403408
.doOnSuccess(messages -> {
409+
messages = Objects.requireNonNullElse(messages, Map.of());
404410
readSize.set(messages.size());
405411
log.debug("Successfully read from stream, size '{}'", messages.size());
406412
})
@@ -434,9 +440,23 @@ private Mono<Map<StreamMessageId, Map<String, M>>> recoverFromNoGroup() {
434440
private Mono<ProcessingResult> processMessage(Map.Entry<StreamMessageId, Map<String, M>> entry) {
435441
var messageId = entry.getKey();
436442
log.info("Message received with messageId '{}'", messageId);
437-
var message = Optional.ofNullable(entry.getValue())
438-
.map(valueMap -> valueMap.get(payloadField))
439-
.orElse(null);
443+
M message;
444+
try {
445+
message = Optional.ofNullable(entry.getValue())
446+
.map(valueMap -> valueMap.get(payloadField))
447+
.orElse(null);
448+
} catch (ClassCastException classCastException) {
449+
// Fix for OPIK-5647: received Collections.emptyList() as the entry value for empty/malformed stream
450+
// entries, which the generic Map<String, M> type erasure hides at compile time.
451+
// ClassCastException is already in NON_RETRYABLE_EXCEPTIONS,
452+
// so the failure path will ack and remove the message without retry.
453+
// Not logging here — postProcessFailureMessages logs non-retryable errors with full context.
454+
return Mono.just(ProcessingResult.builder()
455+
.messageId(messageId)
456+
.status(MessageStatus.FAILURE)
457+
.error(classCastException)
458+
.build());
459+
}
440460
var startMillis = System.currentTimeMillis();
441461
// Deferring as processEvent is out of our control, it might not return a cold Mono
442462
return Mono.defer(() -> processEvent(message))
@@ -535,7 +555,10 @@ private Mono<Long> ackAndRemoveMessages(List<StreamMessageId> messageIds) {
535555
// Only attempt to remove if ack was successful
536556
.then(stream.remove(idsArray)
537557
.subscribeOn(consumerScheduler))
538-
.doOnSuccess(size -> log.debug("Successfully ack and remove from stream, size '{}'", size))
558+
.doOnSuccess(size -> {
559+
size = Objects.requireNonNullElse(size, 0L);
560+
log.debug("Successfully ack and remove from stream, size '{}'", size);
561+
})
539562
.onErrorResume(throwable -> {
540563
// If ack and or remove fails, message will be automatically claimed and retried
541564
ackAndRemoveErrors.add(1);
@@ -614,7 +637,7 @@ private Mono<PendingEntry> listPending(StreamMessageId messageId) {
614637
.subscribeOn(consumerScheduler)
615638
.filter(CollectionUtils::isNotEmpty)
616639
.map(List::getFirst) // Count is 1, so there would be only the first one
617-
.doOnSuccess(size -> log.debug("Successfully list pending messageId '{}'", messageId))
640+
.doOnNext(pendingEntry -> log.debug("Successfully list pending messageId '{}'", messageId))
618641
.onErrorResume(throwable -> {
619642
listPendingErrors.add(1);
620643
log.warn("Error listing pending messageId '{}'", messageId, throwable);

apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/EvalSuiteAssertionSampler.java

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import com.comet.opik.api.DatasetVersion;
44
import com.comet.opik.api.EvaluatorItem;
5+
import com.comet.opik.api.LlmProvider;
56
import com.comet.opik.api.PromptType;
7+
import com.comet.opik.api.ProviderApiKey;
68
import com.comet.opik.api.Trace;
79
import com.comet.opik.api.evaluators.AutomationRuleEvaluatorType;
810
import com.comet.opik.api.events.TraceToScoreLlmAsJudge;
@@ -11,6 +13,7 @@
1113
import com.comet.opik.domain.DatasetItemService;
1214
import com.comet.opik.domain.DatasetVersionService;
1315
import com.comet.opik.domain.IdGenerator;
16+
import com.comet.opik.domain.LlmProviderApiKeyService;
1417
import com.comet.opik.domain.evaluators.OnlineScorePublisher;
1518
import com.comet.opik.infrastructure.EvalSuiteConfig;
1619
import com.comet.opik.infrastructure.auth.RequestContext;
@@ -21,6 +24,7 @@
2124
import lombok.extern.slf4j.Slf4j;
2225
import reactor.core.publisher.Mono;
2326
import reactor.core.scheduler.Schedulers;
27+
import reactor.util.context.Context;
2428
import ru.vyarus.dropwizard.guice.module.installer.feature.eager.EagerSingleton;
2529
import ru.vyarus.dropwizard.guice.module.yaml.bind.Config;
2630

@@ -30,7 +34,9 @@
3034
import java.util.List;
3135
import java.util.Map;
3236
import java.util.Optional;
37+
import java.util.Set;
3338
import java.util.UUID;
39+
import java.util.stream.Collectors;
3440
import java.util.stream.Stream;
3541

3642
/**
@@ -56,6 +62,7 @@ public class EvalSuiteAssertionSampler {
5662
private final IdGenerator idGenerator;
5763
private final EvalSuiteConfig evalSuiteConfig;
5864
private final EvalSuiteEvaluatorMapper evaluatorMapper;
65+
private final LlmProviderApiKeyService llmProviderApiKeyService;
5966

6067
@Inject
6168
public EvalSuiteAssertionSampler(
@@ -64,13 +71,15 @@ public EvalSuiteAssertionSampler(
6471
@NonNull OnlineScorePublisher onlineScorePublisher,
6572
@NonNull IdGenerator idGenerator,
6673
@NonNull @Config("evalSuite") EvalSuiteConfig evalSuiteConfig,
67-
@NonNull EvalSuiteEvaluatorMapper evaluatorMapper) {
74+
@NonNull EvalSuiteEvaluatorMapper evaluatorMapper,
75+
@NonNull LlmProviderApiKeyService llmProviderApiKeyService) {
6876
this.datasetItemService = datasetItemService;
6977
this.datasetVersionService = datasetVersionService;
7078
this.onlineScorePublisher = onlineScorePublisher;
7179
this.idGenerator = idGenerator;
7280
this.evalSuiteConfig = evalSuiteConfig;
7381
this.evaluatorMapper = evaluatorMapper;
82+
this.llmProviderApiKeyService = llmProviderApiKeyService;
7483
}
7584

7685
@Subscribe
@@ -83,13 +92,26 @@ public void onTracesCreated(TracesCreated tracesBatch) {
8392
return;
8493
}
8594

86-
var reactiveContext = reactor.util.context.Context.of(
95+
var reactiveContext = Context.of(
8796
RequestContext.WORKSPACE_ID, tracesBatch.workspaceId(),
8897
RequestContext.USER_NAME, tracesBatch.userName(),
8998
RequestContext.VISIBILITY, com.comet.opik.api.Visibility.PRIVATE);
9099

91100
Duration fetchTimeout = Duration.ofSeconds(evalSuiteConfig.getFetchTimeoutSeconds());
92101

102+
// Resolve model once per batch: prefer connected provider, fall back to first trace's model
103+
var connectedProviders = getConnectedProviders(tracesBatch.workspaceId());
104+
String modelName = SupportedJudgeProvider.resolveModel(connectedProviders)
105+
.or(() -> getMetadataString(completeTraces.getFirst(), "eval_suite_model"))
106+
.orElse(null);
107+
108+
if (modelName == null) {
109+
log.warn("No LLM model resolved for eval suite batch in workspace '{}' — "
110+
+ "no supported provider connected and no eval_suite_model in trace metadata",
111+
tracesBatch.workspaceId());
112+
return;
113+
}
114+
93115
// Cache dataset evaluators by (datasetId:versionHash) to avoid redundant fetches
94116
Map<String, List<PreparedEvaluator>> datasetEvaluatorsCache = new HashMap<>();
95117

@@ -120,7 +142,7 @@ public void onTracesCreated(TracesCreated tracesBatch) {
120142
.contextWrite(reactiveContext)
121143
.timeout(fetchTimeout)
122144
.block();
123-
return evaluatorMapper.prepareEvaluators(result.evaluators());
145+
return evaluatorMapper.prepareEvaluators(result.evaluators(), modelName);
124146
});
125147

126148
var datasetItemId = getMetadataString(trace, "eval_suite_dataset_item_id");
@@ -134,7 +156,8 @@ public void onTracesCreated(TracesCreated tracesBatch) {
134156
.flatMap(itemId -> {
135157
List<PreparedEvaluator> allEvaluators = new ArrayList<>(
136158
preparedDatasetEvaluators);
137-
allEvaluators.addAll(fetchItemEvaluators(itemId, reactiveContext));
159+
allEvaluators.addAll(fetchItemEvaluators(itemId, reactiveContext,
160+
modelName));
138161

139162
if (allEvaluators.isEmpty()) {
140163
log.debug("No evaluators found for trace '{}', dataset item '{}'",
@@ -191,7 +214,8 @@ private Mono<DatasetEvaluatorsResult> fetchDatasetEvaluators(UUID datasetId, Str
191214
}
192215

193216
private List<PreparedEvaluator> fetchItemEvaluators(
194-
UUID itemId, reactor.util.context.Context reactiveContext) {
217+
UUID itemId, Context reactiveContext,
218+
String modelName) {
195219
try {
196220
var item = datasetItemService.get(itemId)
197221
.contextWrite(reactiveContext)
@@ -202,7 +226,7 @@ private List<PreparedEvaluator> fetchItemEvaluators(
202226
return List.of();
203227
}
204228

205-
return evaluatorMapper.prepareEvaluators(item.evaluators());
229+
return evaluatorMapper.prepareEvaluators(item.evaluators(), modelName);
206230
} catch (Exception e) {
207231
log.error("Failed to fetch evaluators for item '{}'", itemId, e);
208232
return List.of();
@@ -230,4 +254,16 @@ private Optional<UUID> parseUUID(String id, UUID traceId) {
230254
}
231255
}
232256

257+
private Set<LlmProvider> getConnectedProviders(String workspaceId) {
258+
try {
259+
return llmProviderApiKeyService.find(workspaceId)
260+
.content().stream()
261+
.map(ProviderApiKey::provider)
262+
.collect(Collectors.toSet());
263+
} catch (Exception e) {
264+
log.error("Failed to fetch connected providers for workspace '{}'", workspaceId, e);
265+
return Set.of();
266+
}
267+
}
268+
233269
}

apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/EvalSuiteEvaluatorMapper.java

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@
1515
import jakarta.inject.Singleton;
1616
import lombok.NonNull;
1717
import lombok.extern.slf4j.Slf4j;
18-
import org.apache.commons.lang3.StringUtils;
1918

2019
import java.util.ArrayList;
2120
import java.util.HashMap;
2221
import java.util.List;
2322
import java.util.Map;
24-
import java.util.Optional;
2523
import java.util.stream.Collectors;
2624
import java.util.stream.Stream;
2725

@@ -54,7 +52,8 @@ public int getEffectiveRunsPerItem(ExecutionPolicy itemPolicy, ExecutionPolicy v
5452
return evalSuiteConfig.getDefaultRunsPerItem();
5553
}
5654

57-
public List<PreparedEvaluator> prepareEvaluators(List<EvaluatorItem> evaluators) {
55+
public List<PreparedEvaluator> prepareEvaluators(List<EvaluatorItem> evaluators,
56+
String modelName) {
5857
return evaluators.stream()
5958
.filter(evaluator -> {
6059
if (evaluator.type() != EvaluatorType.LLM_JUDGE) {
@@ -66,7 +65,7 @@ public List<PreparedEvaluator> prepareEvaluators(List<EvaluatorItem> evaluators)
6665
})
6766
.flatMap(evaluator -> {
6867
try {
69-
LlmAsJudgeCode code = toScoringCode(evaluator.config());
68+
LlmAsJudgeCode code = toScoringCode(evaluator.config(), modelName);
7069

7170
Map<String, String> scoreNameMapping = code.schema() != null
7271
? code.schema().stream()
@@ -84,16 +83,22 @@ public List<PreparedEvaluator> prepareEvaluators(List<EvaluatorItem> evaluators)
8483
.toList();
8584
}
8685

87-
LlmAsJudgeCode toScoringCode(JsonNode config) {
88-
LlmAsJudgeCode code = deserializeEvaluatorConfig(config);
89-
code = resolveModelName(code);
86+
LlmAsJudgeCode toScoringCode(JsonNode config, String modelName) {
87+
LlmAsJudgeCode code = deserializeScoringCode(config, modelName);
9088
code = renameSchemaToAssertionKeys(code);
9189
code = applyEvalSuitePrompt(code);
9290
return code;
9391
}
9492

95-
private LlmAsJudgeCode deserializeEvaluatorConfig(JsonNode config) {
96-
return JsonUtils.treeToValue(config, LlmAsJudgeCode.class);
93+
private LlmAsJudgeCode deserializeScoringCode(JsonNode config, String modelName) {
94+
var code = JsonUtils.treeToValue(config, LlmAsJudgeCode.class);
95+
var existingModel = code.model();
96+
var model = (existingModel != null ? existingModel.toBuilder() : LlmAsJudgeModelParameters.builder())
97+
.name(modelName)
98+
.build();
99+
return code.toBuilder()
100+
.model(model)
101+
.build();
97102
}
98103

99104
/**
@@ -160,17 +165,4 @@ private String formatAssertions(List<LlmAsJudgeOutputSchema> schema) {
160165
.collect(Collectors.joining("\n"));
161166
}
162167

163-
private LlmAsJudgeCode resolveModelName(LlmAsJudgeCode code) {
164-
var existingModel = Optional.ofNullable(code.model());
165-
if (existingModel.map(LlmAsJudgeModelParameters::name).filter(StringUtils::isNotBlank).isEmpty()) {
166-
var resolvedModel = LlmAsJudgeModelParameters.builder()
167-
.name(evalSuiteConfig.getDefaultModelName())
168-
.temperature(existingModel.map(LlmAsJudgeModelParameters::temperature).orElse(null))
169-
.seed(existingModel.map(LlmAsJudgeModelParameters::seed).orElse(null))
170-
.customParameters(existingModel.map(LlmAsJudgeModelParameters::customParameters).orElse(null))
171-
.build();
172-
return new LlmAsJudgeCode(resolvedModel, code.messages(), code.variables(), code.schema());
173-
}
174-
return code;
175-
}
176168
}

0 commit comments

Comments
 (0)