Skip to content

Commit

Permalink
Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@thiagohora
thiagohora committed Dec 13, 2024
1 parent db2961b commit 5653f7e
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 28 deletions.
13 changes: 3 additions & 10 deletions apps/opik-backend/src/main/java/com/comet/opik/api/Span.java
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package com.comet.opik.api;

import com.comet.opik.domain.SpanType;
import com.comet.opik.utils.DurationUtils;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
Expand Down Expand Up @@ -53,7 +51,9 @@ public record Span(
@JsonView({
Span.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) List<FeedbackScore> feedbackScores,
@JsonView({
Span.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) BigDecimal totalEstimatedCost){
Span.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) BigDecimal totalEstimatedCost,
@JsonView({
Span.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY, description = "Duration in milliseconds as a decimal number to support sub-millisecond precision") Double duration){

public record SpanPage(
@JsonView(Span.View.Public.class) int page,
Expand All @@ -72,11 +72,4 @@ public static class Write {
public static class Public {
}
}

@JsonProperty
@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.getDurationInMillisWithSubMilliPrecision(startTime, endTime);
}
}
12 changes: 3 additions & 9 deletions apps/opik-backend/src/main/java/com/comet/opik/api/Trace.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.comet.opik.api;

import com.comet.opik.utils.DurationUtils;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
Expand Down Expand Up @@ -47,7 +45,9 @@ public record Trace(
@JsonView({
Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) List<FeedbackScore> feedbackScores,
@JsonView({
Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) BigDecimal totalEstimatedCost){
Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) BigDecimal totalEstimatedCost,
@JsonView({
Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY, description = "Duration in milliseconds as a decimal number to support sub-millisecond precision") Double duration){

public record TracePage(
@JsonView(Trace.View.Public.class) int page,
Expand All @@ -68,10 +68,4 @@ public static class Public {
}
}

@JsonProperty
@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.getDurationInMillisWithSubMilliPrecision(startTime, endTime);
}
}
7 changes: 5 additions & 2 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/SpanDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,8 @@ LEFT JOIN (

private static final String SELECT_BY_ID = """
SELECT
*
*,
duration
FROM
spans
WHERE id = :id
Expand Down Expand Up @@ -493,7 +494,8 @@ LEFT JOIN (
created_at,
last_updated_at,
created_by,
last_updated_by
last_updated_by,
duration
FROM spans
WHERE project_id = :project_id
AND workspace_id = :workspace_id
Expand Down Expand Up @@ -1060,6 +1062,7 @@ private Publisher<Span> mapToDto(Result result) {
.lastUpdatedAt(row.get("last_updated_at", Instant.class))
.createdBy(row.get("created_by", String.class))
.lastUpdatedBy(row.get("last_updated_by", String.class))
.duration(row.get("duration", Double.class))
.build();
});
}
Expand Down
15 changes: 11 additions & 4 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,13 @@ INSERT INTO traces (
private static final String SELECT_BY_ID = """
SELECT
t.*,
t.duration,
sumMap(s.usage) as usage,
sum(s.total_estimated_cost) as total_estimated_cost
FROM (
SELECT
*
*,
duration
FROM traces
WHERE workspace_id = :workspace_id
AND id = :id
Expand All @@ -280,14 +282,16 @@ LEFT JOIN (
LIMIT 1 BY id
) AS s ON t.id = s.trace_id
GROUP BY
t.*
t.*,
duration
ORDER BY t.id DESC
;
""";

private static final String SELECT_BY_PROJECT_ID = """
SELECT
t.*,
t.duration,
sumMap(s.usage) as usage,
sum(s.total_estimated_cost) as total_estimated_cost
FROM (
Expand All @@ -305,7 +309,8 @@ LEFT JOIN (
created_at,
last_updated_at,
created_by,
last_updated_by
last_updated_by,
duration
FROM traces
WHERE project_id = :project_id
AND workspace_id = :workspace_id
Expand Down Expand Up @@ -343,7 +348,8 @@ LEFT JOIN (
LIMIT 1 BY id
) AS s ON t.id = s.trace_id
GROUP BY
t.*
t.*,
t.duration
<if(trace_aggregation_filters)>
HAVING <trace_aggregation_filters>
<endif>
Expand Down Expand Up @@ -901,6 +907,7 @@ private Publisher<Trace> mapToDto(Result result) {
.lastUpdatedAt(row.get("last_updated_at", Instant.class))
.createdBy(row.get("created_by", String.class))
.lastUpdatedBy(row.get("last_updated_by", String.class))
.duration(row.get("duration", Double.class))
.build());
}

Expand Down
28 changes: 27 additions & 1 deletion 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 @@ -37,6 +37,7 @@
import com.comet.opik.domain.cost.ModelPrice;
import com.comet.opik.infrastructure.auth.RequestContext;
import com.comet.opik.podam.PodamFactoryUtils;
import com.comet.opik.utils.DurationUtils;
import com.comet.opik.utils.JsonUtils;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.NullNode;
Expand All @@ -55,6 +56,7 @@
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.HttpStatus;
import org.assertj.core.api.SoftAssertions;
import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
import org.jdbi.v3.core.Jdbi;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -116,14 +118,15 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.within;
import static org.junit.jupiter.params.provider.Arguments.arguments;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class SpansResourceTest {

public static final String URL_TEMPLATE = "%s/v1/private/spans";
public static final String[] IGNORED_FIELDS = {"projectId", "projectName", "createdAt",
"lastUpdatedAt", "feedbackScores", "createdBy", "lastUpdatedBy", "totalEstimatedCost"};
"lastUpdatedAt", "feedbackScores", "createdBy", "lastUpdatedBy", "totalEstimatedCost", "duration"};
public static final String[] IGNORED_FIELDS_SCORES = {"createdAt", "lastUpdatedAt", "createdBy", "lastUpdatedBy"};

public static final String API_KEY = UUID.randomUUID().toString();
Expand Down Expand Up @@ -899,6 +902,8 @@ void findWithImageTruncation(JsonNode original, JsonNode expected, boolean trunc
.input(expected)
.output(expected)
.metadata(expected)
.duration(DurationUtils.getDurationInMillisWithSubMilliPrecision(span.startTime(),
span.endTime()))
.build())
.toList();

Expand Down Expand Up @@ -3385,6 +3390,17 @@ private void assertIgnoredFields(List<Span> actualSpans, List<Span> expectedSpan
.build())
.isEqualTo(expectedFeedbackScores);

SoftAssertions.assertSoftly(softly -> {
var expected = DurationUtils.getDurationInMillisWithSubMilliPrecision(
expectedSpan.startTime(), expectedSpan.endTime());

if (actualSpan.duration() == null || expected == null) {
softly.assertThat(actualSpan.duration()).isEqualTo(expected);
} else {
softly.assertThat(actualSpan.duration()).isEqualTo(expected, within(0.001));
}
});

if (actualSpan.feedbackScores() != null) {
actualSpan.feedbackScores().forEach(feedbackScore -> {
assertThat(feedbackScore.createdAt()).isAfter(expectedSpan.createdAt());
Expand Down Expand Up @@ -3805,6 +3821,16 @@ private Span getAndAssert(Span expectedSpan, String apiKey, String workspaceName
assertThat(actualSpan.lastUpdatedAt()).isAfter(expectedSpan.lastUpdatedAt());
assertThat(actualSpan.createdBy()).isEqualTo(USER);
assertThat(actualSpan.lastUpdatedBy()).isEqualTo(USER);
SoftAssertions.assertSoftly(softly -> {
var expected = DurationUtils.getDurationInMillisWithSubMilliPrecision(
expectedSpan.startTime(), expectedSpan.endTime());

if (actualSpan.duration() == null || expected == null) {
softly.assertThat(actualSpan.duration()).isEqualTo(expected);
} else {
softly.assertThat(actualSpan.duration()).isEqualTo(expected, within(0.001));
}
});
return actualSpan;
}
}
Expand Down
29 changes: 28 additions & 1 deletion apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/TracesResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.comet.opik.domain.cost.ModelPrice;
import com.comet.opik.infrastructure.auth.RequestContext;
import com.comet.opik.podam.PodamFactoryUtils;
import com.comet.opik.utils.DurationUtils;
import com.comet.opik.utils.JsonUtils;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.uuid.Generators;
Expand All @@ -52,6 +53,7 @@
import jakarta.ws.rs.core.Response;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.http.HttpStatus;
import org.assertj.core.api.SoftAssertions;
import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
import org.jdbi.v3.core.Jdbi;
import org.junit.jupiter.api.AfterAll;
Expand Down Expand Up @@ -109,6 +111,7 @@
import static com.github.tomakehurst.wiremock.client.WireMock.post;
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.within;
import static org.junit.jupiter.params.provider.Arguments.arguments;

@DisplayName("Traces Resource Test")
Expand All @@ -118,7 +121,7 @@ class TracesResourceTest {
public static final String URL_TEMPLATE = "%s/v1/private/traces";
private static final String URL_TEMPLATE_SPANS = "%s/v1/private/spans";
private static final String[] IGNORED_FIELDS_TRACES = {"projectId", "projectName", "createdAt",
"lastUpdatedAt", "feedbackScores", "createdBy", "lastUpdatedBy", "totalEstimatedCost"};
"lastUpdatedAt", "feedbackScores", "createdBy", "lastUpdatedBy", "totalEstimatedCost", "duration"};
private static final String[] IGNORED_FIELDS_SPANS = SpansResourceTest.IGNORED_FIELDS;
private static final String[] IGNORED_FIELDS_SCORES = {"createdAt", "lastUpdatedAt", "createdBy", "lastUpdatedBy"};

Expand Down Expand Up @@ -882,6 +885,8 @@ void findWithImageTruncation(JsonNode original, JsonNode expected, boolean trunc
.input(expected)
.output(expected)
.metadata(expected)
.duration(DurationUtils.getDurationInMillisWithSubMilliPrecision(trace.startTime(),
trace.endTime()))
.build())
.toList();

Expand Down Expand Up @@ -3353,6 +3358,18 @@ private static void assertIgnoredFields(Trace actualTrace, Trace expectedTrace)
assertThat(actualTrace.lastUpdatedAt()).isAfter(expectedTrace.lastUpdatedAt());
assertThat(actualTrace.createdBy()).isEqualTo(USER);
assertThat(actualTrace.lastUpdatedBy()).isEqualTo(USER);

SoftAssertions.assertSoftly(softly -> {
var expected = DurationUtils.getDurationInMillisWithSubMilliPrecision(
expectedTrace.startTime(), expectedTrace.endTime());

if (actualTrace.duration() == null || expected == null) {
softly.assertThat(actualTrace.duration()).isEqualTo(expected);
} else {
softly.assertThat(actualTrace.duration()).isEqualTo(expected, within(0.001));
}
});

assertThat(actualTrace.feedbackScores())
.usingRecursiveComparison()
.withComparatorForType(BigDecimal::compareTo, BigDecimal.class)
Expand All @@ -3378,6 +3395,16 @@ private void assertIgnoredFieldsSpans(List<Span> actualSpans, List<Span> expecte
assertThat(actualSpan.projectName()).isNull();
assertThat(actualSpan.createdAt()).isAfter(expectedSpan.createdAt());
assertThat(actualSpan.lastUpdatedAt()).isAfter(expectedSpan.lastUpdatedAt());
SoftAssertions.assertSoftly(softly -> {
var expected = DurationUtils.getDurationInMillisWithSubMilliPrecision(
expectedSpan.startTime(), expectedSpan.endTime());

if (actualSpan.duration() == null || expected == null) {
softly.assertThat(actualSpan.duration()).isEqualTo(expected);
} else {
softly.assertThat(actualSpan.duration()).isEqualTo(expected, within(0.001));
}
});
assertThat(actualSpan.feedbackScores())
.usingRecursiveComparison()
.withComparatorForType(BigDecimal::compareTo, BigDecimal.class)
Expand Down
2 changes: 1 addition & 1 deletion .../test/java/com/comet/opik/podam/manufacturer/NumericalFeedbackDetailTypeManufacturer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import java.math.RoundingMode;

import static com.comet.opik.api.FeedbackDefinition.NumericalFeedbackDefinition.NumericalFeedbackDetail;
import static com.comet.opik.utils.ValidationUtils.*;
import static com.comet.opik.utils.ValidationUtils.SCALE;

public class NumericalFeedbackDetailTypeManufacturer extends AbstractTypeManufacturer<NumericalFeedbackDetail> {

Expand Down

0 comments on commit 5653f7e

Please sign in to comment.