Skip to content

Commit

Permalink
Fix pr review
Browse files Browse the repository at this point in the history
  • Loading branch information
@thiagohora
thiagohora committed Dec 13, 2024
1 parent 1a0a23d commit 57dd59b
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 38 deletions.
2 changes: 1 addition & 1 deletion apps/opik-backend/src/main/java/com/comet/opik/api/Span.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@ public static class Public {
@JsonView({Span.View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY, description = "Duration in milliseconds as a decimal number to support sub-millisecond precision")
public Double duration() {
return DurationUtils.getDurationInSeconds(startTime, endTime);
return DurationUtils.getDurationInMillisWithSubMilliPrecision(startTime, endTime);
}
}
2 changes: 1 addition & 1 deletion apps/opik-backend/src/main/java/com/comet/opik/api/Trace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ public static class Public {
@JsonView({Span.View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY, description = "Duration in milliseconds as a decimal number to support sub-millisecond precision")
public Double duration() {
return DurationUtils.getDurationInSeconds(startTime, endTime);
return DurationUtils.getDurationInMillisWithSubMilliPrecision(startTime, endTime);
}
}
2 changes: 1 addition & 1 deletion apps/opik-backend/src/main/java/com/comet/opik/utils/DurationUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class DurationUtils {

public static final Double TIME_UNIT = 1_000.0;

public static Double getDurationInSeconds(@NonNull Instant startTime, Instant endTime) {
public static Double getDurationInMillisWithSubMilliPrecision(@NonNull Instant startTime, Instant endTime) {
if (endTime == null) {
return null;
}
Expand Down
140 changes: 123 additions & 17 deletions apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2770,26 +2770,25 @@ void getSpansByProject__whenFilterFeedbackScoresLessThanEqual__thenReturnSpansFi

Stream<Arguments> getSpansByProject__whenFilterByDuration__thenReturnSpansFiltered() {
return Stream.of(
arguments(Operator.EQUAL, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.GREATER_THAN, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.GREATER_THAN,
Duration.ofMillis(8L).toNanos() / 1000, 7.0),
arguments(Operator.GREATER_THAN_EQUAL, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.GREATER_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.GREATER_THAN_EQUAL, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.GREATER_THAN_EQUAL,
Duration.ofMillis(1L).plusNanos(1000).toNanos() / 1000, 1.0),
arguments(Operator.LESS_THAN, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.LESS_THAN,
Duration.ofMillis(1L).plusNanos(1).toNanos() / 1000, 2.0),
arguments(Operator.LESS_THAN_EQUAL, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.LESS_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.LESS_THAN_EQUAL, Instant.now().truncatedTo(ChronoUnit.MILLIS),
arguments(Operator.LESS_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 2.0));
}

@ParameterizedTest
@MethodSource
void getSpansByProject__whenFilterByDuration__thenReturnSpansFiltered(Operator operator, Instant start,
long end, double duration) {
void getSpansByProject__whenFilterByDuration__thenReturnSpansFiltered(Operator operator, long end, double duration) {
String workspaceName = UUID.randomUUID().toString();
String workspaceId = UUID.randomUUID().toString();
String apiKey = UUID.randomUUID().toString();
Expand All @@ -2813,6 +2812,7 @@ void getSpansByProject__whenFilterByDuration__thenReturnSpansFiltered(Operator o
})
.collect(Collectors.toCollection(ArrayList::new));

var start = Instant.now().truncatedTo(ChronoUnit.MILLIS);
spans.set(0, spans.getFirst().toBuilder()
.startTime(start)
.endTime(start.plus(end, ChronoUnit.MICROS))
Expand Down Expand Up @@ -3133,7 +3133,7 @@ static Stream<Filter> getSpansByProject__whenFilterInvalidOperatorForFieldType__
@MethodSource
void getSpansByProject__whenFilterInvalidOperatorForFieldType__thenReturn400(Filter filter) {
var expectedError = new io.dropwizard.jersey.errors.ErrorMessage(
400,
HttpStatus.SC_BAD_REQUEST,
"Invalid operator '%s' for field '%s' of type '%s'".formatted(
filter.operator().getQueryParamOperator(),
filter.field().getQueryParamField(),
Expand All @@ -3148,7 +3148,7 @@ void getSpansByProject__whenFilterInvalidOperatorForFieldType__thenReturn400(Fil
.header(WORKSPACE_HEADER, TEST_WORKSPACE)
.get();

assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(400);
assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST);

var actualError = actualResponse.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class);
assertThat(actualError).isEqualTo(expectedError);
Expand Down Expand Up @@ -3252,7 +3252,7 @@ void getSpansByProject__whenFilterInvalidValueOrKeyForFieldType__thenReturn400(F
mockTargetWorkspace(apiKey, workspaceName, workspaceId);

var expectedError = new io.dropwizard.jersey.errors.ErrorMessage(
400,
HttpStatus.SC_BAD_REQUEST,
"Invalid value '%s' or key '%s' for field '%s' of type '%s'".formatted(
filter.value(),
filter.key(),
Expand All @@ -3269,7 +3269,7 @@ void getSpansByProject__whenFilterInvalidValueOrKeyForFieldType__thenReturn400(F
.header(WORKSPACE_HEADER, workspaceName)
.get();

assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(400);
assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST);

var actualError = actualResponse.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class);
assertThat(actualError).isEqualTo(expectedError);
Expand Down Expand Up @@ -7227,6 +7227,80 @@ void getSpanStats__whenFilterFeedbackScoresLessThanEqual__thenReturnSpansFiltere
getStatsAndAssert(projectName, null, filters, null, null, apiKey, workspaceName, projectStatItems);
}

Stream<Arguments> getSpanStats__whenFilterByDuration__thenReturnSpansFiltered() {
return Stream.of(
arguments(Operator.EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.GREATER_THAN,
Duration.ofMillis(8L).toNanos() / 1000, 7.0),
arguments(Operator.GREATER_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.GREATER_THAN_EQUAL,
Duration.ofMillis(1L).plusNanos(1000).toNanos() / 1000, 1.0),
arguments(Operator.LESS_THAN,
Duration.ofMillis(1L).plusNanos(1).toNanos() / 1000, 2.0),
arguments(Operator.LESS_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 1.0),
arguments(Operator.LESS_THAN_EQUAL,
Duration.ofMillis(1L).toNanos() / 1000, 2.0));
}

@ParameterizedTest
@MethodSource
void getSpanStats__whenFilterByDuration__thenReturnSpansFiltered(Operator operator, long end, double duration) {
String workspaceName = UUID.randomUUID().toString();
String workspaceId = UUID.randomUUID().toString();
String apiKey = UUID.randomUUID().toString();

mockTargetWorkspace(apiKey, workspaceName, workspaceId);

var projectName = generator.generate().toString();
var spans = PodamFactoryUtils.manufacturePojoList(podamFactory, Span.class)
.stream()
.map(span -> {
Instant now = Instant.now();
return span.toBuilder()
.projectId(null)
.projectName(projectName)
.feedbackScores(null)
.startTime(now)
.endTime(Set.of(Operator.LESS_THAN, Operator.LESS_THAN_EQUAL).contains(operator)
? Instant.now().plusSeconds(2)
: now.plusNanos(1000))
.build();
})
.collect(Collectors.toCollection(ArrayList::new));

var start = Instant.now().truncatedTo(ChronoUnit.MILLIS);
spans.set(0, spans.getFirst().toBuilder()
.startTime(start)
.endTime(start.plus(end, ChronoUnit.MICROS))
.build());

spans.forEach(expectedSpan -> createAndAssert(expectedSpan, apiKey, workspaceName));

var expectedSpans = List.of(spans.getFirst());

var unexpectedSpans = PodamFactoryUtils.manufacturePojoList(podamFactory, Span.class).stream()
.map(span -> span.toBuilder()
.projectId(null)
.build())
.toList();

unexpectedSpans.forEach(expectedSpan -> createAndAssert(expectedSpan, apiKey, workspaceName));

var filters = List.of(
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(operator)
.value(String.valueOf(duration))
.build());

List<ProjectStatItem<?>> stats = getProjectSpanStatItems(expectedSpans);

getStatsAndAssert(projectName, null, filters, null, null, apiKey, workspaceName, stats);
}

static Stream<Filter> getSpanStats__whenFilterInvalidOperatorForFieldType__thenReturn400() {
return Stream.of(
SpanFilter.builder()
Expand Down Expand Up @@ -7493,14 +7567,35 @@ static Stream<Filter> getSpanStats__whenFilterInvalidOperatorForFieldType__thenR
.field(SpanField.TAGS)
.operator(Operator.LESS_THAN_EQUAL)
.value(RandomStringUtils.randomAlphanumeric(10))
.build());
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.NOT_CONTAINS)
.value("1")
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.CONTAINS)
.value("1")
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.ENDS_WITH)
.value("1")
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.STARTS_WITH)
.value("1")
.build()
);
}

@ParameterizedTest
@MethodSource
void getSpanStats__whenFilterInvalidOperatorForFieldType__thenReturn400(Filter filter) {
var expectedError = new io.dropwizard.jersey.errors.ErrorMessage(
400,
HttpStatus.SC_BAD_REQUEST,
"Invalid operator '%s' for field '%s' of type '%s'".formatted(
filter.operator().getQueryParamOperator(),
filter.field().getQueryParamField(),
Expand Down Expand Up @@ -7597,7 +7692,18 @@ static Stream<Filter> getSpanStats__whenFilterInvalidValueOrKeyForFieldType__the
.operator(Operator.EQUAL)
.value("")
.key("hallucination")
.build());
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.EQUAL)
.value("")
.build(),
SpanFilter.builder()
.field(SpanField.DURATION)
.operator(Operator.EQUAL)
.value(RandomStringUtils.randomAlphanumeric(5))
.build()
);
}

@ParameterizedTest
Expand All @@ -7610,7 +7716,7 @@ void getSpanStats__whenFilterInvalidValueOrKeyForFieldType__thenReturn400(Filter
mockTargetWorkspace(apiKey, workspaceName, workspaceId);

var expectedError = new io.dropwizard.jersey.errors.ErrorMessage(
400,
HttpStatus.SC_BAD_REQUEST,
"Invalid value '%s' or key '%s' for field '%s' of type '%s'".formatted(
filter.value(),
filter.key(),
Expand Down
Loading

0 comments on commit 57dd59b

Please sign in to comment.