Skip to content

Commit f8a0100

Browse files
committed
pull latest + fix conflicts
2 parents 9ae20ca + 4da5478 commit f8a0100

5 files changed

Lines changed: 96 additions & 32 deletions

File tree

lib/sdk/server-ai/src/main/java/com/launchdarkly/sdk/server/ai/LDAIClient.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ AIJudgeConfig judgeConfig(
9090
* stores the resumption token from a previous tracker (via
9191
* {@link LDAIConfigTracker#getResumptionToken()}) and passes it back here to continue tracking
9292
* against the same run.
93+
* <p>
94+
* <strong>Security note:</strong> resumption tokens embed flag-evaluation details such as the
95+
* variation key and config version. Keep tokens server-side and do not round-trip them through
96+
* untrusted clients where they could leak flag-targeting information.
9397
*
9498
* @param resumptionToken the token returned by a previous tracker; must not be {@code null}
9599
* @param context the evaluation context for the new request; must not be {@code null}

lib/sdk/server-ai/src/main/java/com/launchdarkly/sdk/server/ai/datamodel/LDAITrackingTypes.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,21 +423,29 @@ public Builder sampled(boolean sampled) {
423423
/**
424424
* Sets the metric key.
425425
*
426-
* @param metricKey the metric key; may be {@code null}
426+
* @param metricKey the metric key; may be {@code null}, but must not be blank if non-null
427427
* @return this builder
428+
* @throws IllegalArgumentException if {@code metricKey} is non-null and blank
428429
*/
429430
public Builder metricKey(String metricKey) {
431+
if (metricKey != null && metricKey.trim().isEmpty()) {
432+
throw new IllegalArgumentException("metricKey must not be blank");
433+
}
430434
this.metricKey = metricKey;
431435
return this;
432436
}
433437

434438
/**
435439
* Sets the judge score.
436440
*
437-
* @param score the score; may be {@code null}
441+
* @param score the score; may be {@code null}, but must be finite if non-null
438442
* @return this builder
443+
* @throws IllegalArgumentException if {@code score} is non-null and non-finite (NaN or infinite)
439444
*/
440445
public Builder score(Double score) {
446+
if (score != null && !Double.isFinite(score)) {
447+
throw new IllegalArgumentException("score must be finite");
448+
}
441449
this.score = score;
442450
return this;
443451
}

lib/sdk/server-ai/src/main/java/com/launchdarkly/sdk/server/ai/internal/LDAIConfigTrackerImpl.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public String getResumptionToken() {
155155
@Override
156156
public void trackDuration(Duration duration) {
157157
if (duration == null) {
158-
logger.warn("Skipping trackDuration: duration was null.");
158+
logger.debug("Skipping trackDuration: duration was null.");
159159
return;
160160
}
161161
long ms = Math.max(0L, duration.toMillis());
@@ -181,7 +181,7 @@ public <T> T trackDurationOf(Callable<T> operation) throws Exception {
181181
@Override
182182
public void trackTimeToFirstToken(Duration duration) {
183183
if (duration == null) {
184-
logger.warn("Skipping trackTimeToFirstToken: duration was null.");
184+
logger.debug("Skipping trackTimeToFirstToken: duration was null.");
185185
return;
186186
}
187187
long ms = Math.max(0L, duration.toMillis());
@@ -213,7 +213,7 @@ public void trackError() {
213213
@Override
214214
public void trackFeedback(FeedbackKind kind) {
215215
if (kind == null) {
216-
logger.warn("Skipping trackFeedback: kind was null.");
216+
logger.debug("Skipping trackFeedback: kind was null.");
217217
return;
218218
}
219219
// Resolve event name BEFORE claiming the guard — an exception here must not burn the slot.
@@ -228,7 +228,7 @@ public void trackFeedback(FeedbackKind kind) {
228228
@Override
229229
public void trackTokens(TokenUsage tokens) {
230230
if (tokens == null) {
231-
logger.warn("Skipping trackTokens: tokens was null.");
231+
logger.debug("Skipping trackTokens: tokens was null.");
232232
return;
233233
}
234234
boolean hasPositive = tokens.getTotal() > 0 || tokens.getInput() > 0 || tokens.getOutput() > 0;
@@ -254,7 +254,7 @@ public void trackTokens(TokenUsage tokens) {
254254
@Override
255255
public void trackToolCall(String toolKey) {
256256
if (toolKey == null) {
257-
logger.warn("Skipping trackToolCall: toolKey was null.");
257+
logger.debug("Skipping trackToolCall: toolKey was null.");
258258
return;
259259
}
260260
toolCalls.add(toolKey);
@@ -275,7 +275,7 @@ public void trackToolCalls(List<String> toolKeys) {
275275
@Override
276276
public void trackJudgeResult(JudgeResult result) {
277277
if (result == null) {
278-
logger.warn("Skipping trackJudgeResult: result was null.");
278+
logger.debug("Skipping trackJudgeResult: result was null.");
279279
return;
280280
}
281281
if (!result.isSampled()) {
@@ -284,10 +284,10 @@ public void trackJudgeResult(JudgeResult result) {
284284
if (!result.isSuccess()) {
285285
return;
286286
}
287-
if (result.getMetricKey() == null) {
287+
if (result.getMetricKey() == null || result.getMetricKey().trim().isEmpty()) {
288288
return;
289289
}
290-
if (result.getScore() == null) {
290+
if (result.getScore() == null || !Double.isFinite(result.getScore())) {
291291
return;
292292
}
293293
ObjectBuilder data = baseData();
@@ -315,17 +315,25 @@ public <T> T trackMetricsOf(
315315
trackError();
316316
throw e;
317317
}
318+
// Capture operation duration immediately so a slow extractor does not inflate the metric.
319+
long operationElapsedMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
318320

319-
// Extractor exceptions propagate to the caller — do NOT catch them here.
320-
// Do NOT call trackError() on extractor failure; the AI operation itself succeeded.
321-
AIMetrics metrics = metricsExtractor.apply(result);
321+
// Extractor exceptions propagate to the caller, but the operation's duration must still be
322+
// recorded — the AI operation itself succeeded, only the user-supplied extractor failed.
323+
// Do NOT call trackError(); that signals the operation failed, which is not what happened.
324+
AIMetrics metrics;
325+
try {
326+
metrics = Objects.requireNonNull(metricsExtractor.apply(result), "metricsExtractor returned null");
327+
} catch (RuntimeException e) {
328+
trackDuration(Duration.ofMillis(operationElapsedMs));
329+
throw e;
330+
}
322331

323332
// Duration: prefer runner-reported value (§1.1.13.2), fall back to wall-clock.
324333
if (metrics.getDurationMs() != null) {
325334
trackDuration(Duration.ofMillis(metrics.getDurationMs()));
326335
} else {
327-
long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
328-
trackDuration(Duration.ofMillis(elapsed));
336+
trackDuration(Duration.ofMillis(operationElapsedMs));
329337
}
330338

331339
if (metrics.isSuccess()) {

lib/sdk/server-ai/src/main/java/com/launchdarkly/sdk/server/ai/internal/ResumptionTokens.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
* <p>
1414
* This class is an internal implementation detail and is not part of the supported API.
1515
*/
16+
1617
public final class ResumptionTokens {
17-
private static final int MAX_TOKEN_BYTES = 4096;
18+
private static final int MAX_TOKEN_LENGTH = 4096;
1819
private static final Base64.Encoder ENCODER = Base64.getUrlEncoder().withoutPadding();
1920
private static final Base64.Decoder DECODER = Base64.getUrlDecoder();
2021

@@ -61,8 +62,8 @@ static Decoded decode(String token) {
6162
if (token == null) {
6263
throw new IllegalArgumentException("Resumption token must not be null");
6364
}
64-
if (token.length() > MAX_TOKEN_BYTES) {
65-
throw new IllegalArgumentException("Resumption token exceeds maximum length of " + MAX_TOKEN_BYTES + " bytes");
65+
if (token.length() > MAX_TOKEN_LENGTH) {
66+
throw new IllegalArgumentException("Resumption token exceeds maximum length of " + MAX_TOKEN_LENGTH + " characters");
6667
}
6768

6869
String json;
@@ -150,10 +151,10 @@ private static Decoded parseJson(String json) {
150151
}
151152
}
152153

153-
if (runId == null) {
154+
if (runId == null || runId.isEmpty()) {
154155
throw new IllegalArgumentException("Resumption token missing required field 'runId'");
155156
}
156-
if (configKey == null) {
157+
if (configKey == null || configKey.isEmpty()) {
157158
throw new IllegalArgumentException("Resumption token missing required field 'configKey'");
158159
}
159160
if (version == null) {

lib/sdk/server-ai/src/test/java/com/launchdarkly/sdk/server/ai/internal/LDAIConfigTrackerImplTest.java

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static org.hamcrest.MatcherAssert.assertThat;
44
import static org.hamcrest.Matchers.containsInAnyOrder;
55
import static org.hamcrest.Matchers.containsString;
6+
import static org.hamcrest.Matchers.empty;
67
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
78
import static org.hamcrest.Matchers.is;
89
import static org.hamcrest.Matchers.notNullValue;
@@ -82,6 +83,13 @@ private List<String> warnings() {
8283
.collect(Collectors.toList());
8384
}
8485

86+
private List<String> debugs() {
87+
return logCapture.getMessages().stream()
88+
.filter(m -> m.getLevel() == LDLogLevel.DEBUG)
89+
.map(LogCapture.Message::getText)
90+
.collect(Collectors.toList());
91+
}
92+
8593
private LDValue baseExpectedData() {
8694
return LDValue.buildObject()
8795
.put("runId", RUN_ID)
@@ -172,10 +180,11 @@ public void trackDurationAtMostOnce() {
172180
}
173181

174182
@Test
175-
public void trackDurationNullIsIgnoredWithWarning() {
183+
public void trackDurationNullIsIgnoredWithDebugLog() {
176184
tracker.trackDuration(null);
177185
verify(client, never()).trackMetric(eq("$ld:ai:duration:total"), any(), any(), anyDouble());
178-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
186+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
187+
assertThat(warnings(), is(empty()));
179188
}
180189

181190
// ---- trackDurationOf ------------------------------------------------------
@@ -215,10 +224,11 @@ public void trackTimeToFirstTokenAtMostOnce() {
215224
}
216225

217226
@Test
218-
public void trackTimeToFirstTokenNullIsIgnoredWithWarning() {
227+
public void trackTimeToFirstTokenNullIsIgnoredWithDebugLog() {
219228
tracker.trackTimeToFirstToken(null);
220229
verify(client, never()).trackMetric(eq("$ld:ai:tokens:ttf"), any(), any(), anyDouble());
221-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
230+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
231+
assertThat(warnings(), is(empty()));
222232
}
223233

224234
// ---- trackSuccess / trackError --------------------------------------------
@@ -299,9 +309,10 @@ public void trackFeedbackAtMostOnce() {
299309
}
300310

301311
@Test
302-
public void trackFeedbackNullIsIgnoredWithWarning_slotNotBurned() {
312+
public void trackFeedbackNullIsIgnoredWithDebugLog_slotNotBurned() {
303313
tracker.trackFeedback(null);
304-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
314+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
315+
assertThat(warnings(), is(empty()));
305316
// Slot should not be burned — a subsequent valid call should still work
306317
tracker.trackFeedback(FeedbackKind.POSITIVE);
307318
verify(client, times(1)).trackMetric(eq("$ld:ai:feedback:user:positive"), any(), any(), anyDouble());
@@ -342,10 +353,11 @@ public void trackTokensAtMostOnce() {
342353
}
343354

344355
@Test
345-
public void trackTokensNullIsIgnoredWithWarning() {
356+
public void trackTokensNullIsIgnoredWithDebugLog() {
346357
tracker.trackTokens(null);
347358
verify(client, never()).trackMetric(eq("$ld:ai:tokens:total"), any(), any(), anyDouble());
348-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
359+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
360+
assertThat(warnings(), is(empty()));
349361
}
350362

351363
// ---- trackToolCall --------------------------------------------------------
@@ -382,10 +394,11 @@ public void trackToolCallsDelegate() {
382394
}
383395

384396
@Test
385-
public void trackToolCallNullIsIgnoredWithWarning() {
397+
public void trackToolCallNullIsIgnoredWithDebugLog() {
386398
tracker.trackToolCall(null);
387399
verify(client, never()).trackMetric(eq("$ld:ai:tool_call"), any(), any(), anyDouble());
388-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
400+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
401+
assertThat(warnings(), is(empty()));
389402
}
390403

391404
// ---- trackJudgeResult -----------------------------------------------------
@@ -470,9 +483,10 @@ public void trackJudgeResultIsNotAtMostOnce() {
470483
}
471484

472485
@Test
473-
public void trackJudgeResultNullIsIgnoredWithWarning() {
486+
public void trackJudgeResultNullIsIgnoredWithDebugLog() {
474487
tracker.trackJudgeResult(null);
475-
assertThat(warnings().size(), greaterThanOrEqualTo(1));
488+
assertThat(debugs().size(), greaterThanOrEqualTo(1));
489+
assertThat(warnings(), is(empty()));
476490
}
477491

478492
// ---- trackMetricsOf -------------------------------------------------------
@@ -501,6 +515,23 @@ public void trackMetricsOfUsesRunnerReportedDurationWhenPresent() throws Excepti
501515
verify(client).trackMetric(eq("$ld:ai:duration:total"), any(), any(), eq(999.0));
502516
}
503517

518+
@Test
519+
public void trackMetricsOfWallClockDurationExcludesSlowExtractor() throws Exception {
520+
// Operation returns immediately; extractor sleeps. Recorded duration must reflect only the
521+
// operation, not the extractor work.
522+
long extractorSleepMs = 200L;
523+
AIMetrics metrics = AIMetrics.builder().success(true).build();
524+
tracker.trackMetricsOf(
525+
r -> { try { Thread.sleep(extractorSleepMs); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); } return metrics; },
526+
() -> "ok");
527+
ArgumentCaptor<Double> durationCaptor = ArgumentCaptor.forClass(Double.class);
528+
verify(client).trackMetric(eq("$ld:ai:duration:total"), any(), any(), durationCaptor.capture());
529+
assertThat(
530+
"wall-clock duration must not include extractor time",
531+
durationCaptor.getValue() < (double) extractorSleepMs / 2,
532+
is(true));
533+
}
534+
504535
@Test
505536
public void trackMetricsOfTracksErrorAndRethrowsOnOperationException() {
506537
try {
@@ -528,6 +559,18 @@ public void trackMetricsOfExtractorExceptionPropagatesAndDoesNotCallTrackError()
528559
verify(client, never()).trackMetric(eq("$ld:ai:generation:success"), any(), any(), anyDouble());
529560
}
530561

562+
@Test
563+
public void trackMetricsOfRecordsDurationWhenExtractorThrows() {
564+
try {
565+
tracker.trackMetricsOf(
566+
r -> { throw new RuntimeException("extractor failed"); },
567+
() -> "ok");
568+
} catch (Exception e) {
569+
// expected; we care that duration was recorded before the throw
570+
}
571+
verify(client).trackMetric(eq("$ld:ai:duration:total"), any(), any(), anyDouble());
572+
}
573+
531574
@Test
532575
public void trackMetricsOfTracksToolCalls() throws Exception {
533576
AIMetrics metrics = AIMetrics.builder()

0 commit comments

Comments
 (0)