Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPIK-904: Split find project endpoint into two endpoints find and stats #1176

Merged
merged 3 commits into from
Jan 30, 2025
Merged

OPIK-904: Split find project endpoint into two endpoints find and stats #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
535cf34
OPIK-904: Split find project endpoint into two endpoints find and stats
thiagohora Jan 30, 2025
a1eecf2
Merge branch 'main' into thiagohora/OPIK-904_split_find_project_into_…
thiagohora Jan 30, 2025
56099b2
OPIK-904: Fix tests
thiagohora Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions apps/opik-backend/src/main/java/com/comet/opik/api/Project.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,23 @@ public record Project(
@JsonView({
Project.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable Instant lastUpdatedTraceAt,
@JsonView({
Project.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable List<FeedbackScoreAverage> feedbackScores,
Project.View.Detailed.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable List<FeedbackScoreAverage> feedbackScores,
@JsonView({
Project.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable PercentageValues duration,
Project.View.Detailed.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable PercentageValues duration,
@JsonView({
Project.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable Double totalEstimatedCost,
Project.View.Detailed.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable Double totalEstimatedCost,
@JsonView({
Project.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable Map<String, Double> usage){
Project.View.Detailed.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) @Nullable Map<String, Double> usage){

public static class View {
public static class Write {
}

public static class Public {
}

public static class Detailed extends Public {
}
}

public record ProjectPage(
Expand Down
28 changes: 28 additions & 0 deletions apps/opik-backend/src/main/java/com/comet/opik/api/ProjectStatsSummary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
import com.fasterxml.jackson.databind.annotation.JsonNaming;
import lombok.Builder;

import java.util.List;
import java.util.Map;
import java.util.UUID;

@Builder(toBuilder = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public record ProjectStatsSummary(List<ProjectStatsSummaryItem> content) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this could be a lightweight Page object, with page, size on top of the current content and without total. But this works as FE is fine with it. We can always extend it in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can add in the next iteration


@Builder(toBuilder = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public record ProjectStatsSummaryItem(
UUID projectId,
List<FeedbackScoreAverage> feedbackScores,
ProjectStats.PercentageValues duration,
Double totalEstimatedCost,
Map<String, Double> usage) {
}

}
34 changes: 32 additions & 2 deletions apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ProjectsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.comet.opik.api.Project;
import com.comet.opik.api.ProjectCriteria;
import com.comet.opik.api.ProjectRetrieve;
import com.comet.opik.api.ProjectStatsSummary;
import com.comet.opik.api.ProjectUpdate;
import com.comet.opik.api.error.ErrorMessage;
import com.comet.opik.api.metrics.ProjectMetricRequest;
Expand Down Expand Up @@ -68,6 +69,7 @@
@Tag(name = "Projects", description = "Project related resources")
public class ProjectsResource {

private static final String PAGE_SIZE = "10";
private final @NonNull ProjectService projectService;
private final @NonNull Provider<RequestContext> requestContext;
private final @NonNull SortingFactoryProjects sortingFactory;
Expand All @@ -81,7 +83,7 @@ public class ProjectsResource {
@JsonView({Project.View.Public.class})
public Response find(
@QueryParam("page") @Min(1) @DefaultValue("1") int page,
@QueryParam("size") @Min(1) @DefaultValue("10") int size,
@QueryParam("size") @Min(1) @DefaultValue(PAGE_SIZE) int size,
@QueryParam("name") String name,
@QueryParam("sorting") String sorting) {

Expand Down Expand Up @@ -188,7 +190,7 @@ public Response deleteById(@PathParam("id") UUID id) {
@ApiResponse(responseCode = "400", description = "Bad Request", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
@ApiResponse(responseCode = "404", description = "Not Found", content = @Content(schema = @Schema(implementation = ErrorMessage.class)))
})
@JsonView({Project.View.Public.class})
@JsonView({Project.View.Detailed.class})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no test that verifies fields marked with Detailed view, returned by this endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test remains the same since this endpoint didn't change; only the. find was affected

Copy link
Contributor

@BorisTkachenko BorisTkachenko Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, old test also didn't check it. So we basically never verified that those values(now annotated with Detailed) are returned properly by this endpoint. I suppose it should be added to corresponding test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let me check it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

public Response retrieveProject(
@RequestBody(content = @Content(schema = @Schema(implementation = ProjectRetrieve.class))) @Valid ProjectRetrieve retrieve) {
String workspaceId = requestContext.get().getWorkspaceId();
Expand Down Expand Up @@ -268,4 +270,32 @@ private void validate(ProjectMetricRequest request) {
throw new BadRequestException(ERR_START_BEFORE_END);
}
}

@GET
@Path("/stats")
@Operation(operationId = "getProjectStats", summary = "Get Project Stats", description = "Get Project Stats", responses = {
@ApiResponse(responseCode = "200", description = "Project Stats", content = @Content(schema = @Schema(implementation = ProjectStatsSummary.class))),
})
public Response getProjectStats(
@QueryParam("page") @Min(1) @DefaultValue("1") int page,
@QueryParam("size") @Min(1) @DefaultValue(PAGE_SIZE) int size,
@QueryParam("name") String name,
@QueryParam("sorting") String sorting) {

var criteria = ProjectCriteria.builder()
.projectName(name)
.build();

String workspaceId = requestContext.get().getWorkspaceId();

List<SortingField> sortingFields = sortingFactory.newSorting(sorting);

log.info("Find projects stats by '{}' on workspaceId '{}'", criteria, workspaceId);
ProjectStatsSummary projectStatisticsSummary = projectService.getStats(page, size, criteria, sortingFields);
log.info("Found projects stats by '{}', count '{}' on workspaceId '{}'", criteria,
projectStatisticsSummary.content().size(), workspaceId);

return Response.ok().entity(projectStatisticsSummary).build();
}

}
78 changes: 57 additions & 21 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/ProjectService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.comet.opik.api.Project.ProjectPage;
import com.comet.opik.api.ProjectCriteria;
import com.comet.opik.api.ProjectIdLastUpdated;
import com.comet.opik.api.ProjectStatsSummary;
import com.comet.opik.api.ProjectUpdate;
import com.comet.opik.api.error.EntityAlreadyExistsException;
import com.comet.opik.api.error.ErrorMessage;
Expand Down Expand Up @@ -44,6 +45,7 @@
import java.util.stream.Collectors;

import static com.comet.opik.api.ProjectStats.ProjectStatItem;
import static com.comet.opik.api.ProjectStatsSummary.ProjectStatsSummaryItem;
import static com.comet.opik.infrastructure.db.TransactionTemplateAsync.READ_ONLY;
import static com.comet.opik.infrastructure.db.TransactionTemplateAsync.WRITE;
import static java.util.Collections.reverseOrder;
Expand Down Expand Up @@ -82,6 +84,9 @@ public interface ProjectService {
Project retrieveByName(String projectName);

void recordLastUpdatedTrace(String workspaceId, Collection<ProjectIdLastUpdated> lastUpdatedTraces);

ProjectStatsSummary getStats(int page, int size, @NonNull ProjectCriteria criteria,
@NonNull List<SortingField> sortingFields);
}

@Slf4j
Expand Down Expand Up @@ -203,14 +208,36 @@ public Project get(@NonNull UUID id, @NonNull String workspaceId) {
.nonTransaction(connection -> traceDAO.getLastUpdatedTraceAt(Set.of(id), workspaceId, connection))
.block();

Map<UUID, Map<String, Object>> projectStats = getProjectStats(List.of(id), workspaceId);
return project.toBuilder()
.lastUpdatedTraceAt(lastUpdatedTraceAt.get(project.id()))
.build();
}

@Override
public ProjectStatsSummary getStats(int page, int size, @NonNull ProjectCriteria criteria,
@NonNull List<SortingField> sortingFields) {

String workspaceId = requestContext.get().getWorkspaceId();

List<UUID> projectIds = find(page, size, criteria, sortingFields)
.content()
.stream()
.map(Project::id)
.toList();

Map<UUID, Map<String, Object>> projectStats = getProjectStats(projectIds, workspaceId);

return enhanceProject(project, lastUpdatedTraceAt.get(project.id()), projectStats.get(project.id()));
return ProjectStatsSummary.builder()
.content(
projectIds.stream()
.map(projectId -> getStats(projectId, projectStats.get(projectId)))
.toList())
.build();
}

private Project enhanceProject(Project project, Instant lastUpdatedTraceAt, Map<String, Object> projectStats) {
return project.toBuilder()
.lastUpdatedTraceAt(lastUpdatedTraceAt)
private ProjectStatsSummaryItem getStats(UUID projectId, Map<String, Object> projectStats) {
return ProjectStatsSummaryItem.builder()
.projectId(projectId)
.feedbackScores(StatsMapper.getStatsFeedbackScores(projectStats))
.duration(StatsMapper.getStatsDuration(projectStats))
.totalEstimatedCost(StatsMapper.getStatsTotalEstimatedCost(projectStats))
Expand Down Expand Up @@ -285,14 +312,14 @@ public Page<Project> find(int page, int size, @NonNull ProjectCriteria criteria,
return traceDAO.getLastUpdatedTraceAt(projectIds, workspaceId, connection);
}).block();

List<UUID> projectIds = projectRecordSet.content.stream().map(Project::id).toList();

Map<UUID, Map<String, Object>> projectStats = getProjectStats(projectIds, workspaceId);

List<Project> projects = projectRecordSet.content()
.stream()
.map(project -> enhanceProject(project, projectLastUpdatedTraceAtMap.get(project.id()),
projectStats.get(project.id())))
.map(project -> {
Instant lastUpdatedTraceAt = projectLastUpdatedTraceAtMap.get(project.id());
return project.toBuilder()
.lastUpdatedTraceAt(lastUpdatedTraceAt)
.build();
})
.toList();

return new ProjectPage(page, projects.size(), projectRecordSet.total(), projects,
Expand All @@ -303,8 +330,7 @@ private Map<UUID, Map<String, Object>> getProjectStats(List<UUID> projectIds, St
return traceDAO.getStatsByProjectIds(projectIds, workspaceId)
.map(stats -> stats.entrySet().stream()
.map(entry -> {
Map<String, Object> statsMap = entry.getValue()
.stats()
Map<String, Object> statsMap = entry.getValue().stats()
.stream()
.collect(toMap(ProjectStatItem::getName, ProjectStatItem::getValue));

Expand Down Expand Up @@ -342,9 +368,11 @@ private Page<Project> findWithLastTraceSorting(int page, int size, @NonNull Proj
// get last trace for each project id
Set<UUID> allProjectIds = allProjectIdsLastUpdated.stream().map(ProjectIdLastUpdated::id)
.collect(toUnmodifiableSet());

Map<UUID, Instant> projectLastUpdatedTraceAtMap = transactionTemplateAsync
.nonTransaction(connection -> traceDAO.getLastUpdatedTraceAt(allProjectIds, workspaceId, connection))
.block();

if (projectLastUpdatedTraceAtMap == null) {
return ProjectPage.empty(page);
}
Expand All @@ -365,12 +393,12 @@ private Page<Project> findWithLastTraceSorting(int page, int size, @NonNull Proj
return repository.findByIds(new HashSet<>(finalIds), workspaceId);
}).stream().collect(Collectors.toMap(Project::id, Function.identity()));

Map<UUID, Map<String, Object>> projectStats = getProjectStats(finalIds, workspaceId);

// compose the final projects list by the correct order and add last trace to it
List<Project> projects = finalIds.stream().map(projectsById::get)
.map(project -> enhanceProject(project, projectLastUpdatedTraceAtMap.get(project.id()),
projectStats.get(project.id())))
List<Project> projects = finalIds.stream()
.map(projectsById::get)
.map(project -> project.toBuilder()
.lastUpdatedTraceAt(projectLastUpdatedTraceAtMap.get(project.id()))
.build())
.toList();

return new ProjectPage(page, projects.size(), allProjectIdsLastUpdated.size(), projects,
Expand All @@ -389,7 +417,9 @@ private List<UUID> sortByLastTrace(
? reverseOrder(Map.Entry.comparingByValue())
: Map.Entry.comparingByValue();

return projectLastUpdatedTraceAtMap.entrySet().stream().sorted(comparator)
return projectLastUpdatedTraceAtMap.entrySet()
.stream()
.sorted(comparator)
.map(Map.Entry::getKey)
.toList();
}
Expand Down Expand Up @@ -453,8 +483,14 @@ public Project retrieveByName(@NonNull String projectName) {
Map<UUID, Map<String, Object>> projectStats = getProjectStats(List.of(project.id()),
workspaceId);

return enhanceProject(project, projectLastUpdatedTraceAtMap.get(project.id()),
projectStats.get(project.id()));
return project.toBuilder()
.lastUpdatedTraceAt(projectLastUpdatedTraceAtMap.get(project.id()))
.feedbackScores(StatsMapper.getStatsFeedbackScores(projectStats.get(project.id())))
.usage(StatsMapper.getStatsUsage(projectStats.get(project.id())))
.duration(StatsMapper.getStatsDuration(projectStats.get(project.id())))
.totalEstimatedCost(
StatsMapper.getStatsTotalEstimatedCost(projectStats.get(project.id())))
.build();
})
.orElseThrow(this::createNotFoundError);
});
Expand Down
10 changes: 8 additions & 2 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/SpanDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,15 @@ AND notEquals(s.start_time, toDateTime64('1970-01-01 00:00:00.000', 9)),
(dateDiff('microsecond', s.start_time, s.end_time) / 1000.0),
NULL) AS duration_millis,
groupArray(tuple(c.*)) AS comments
FROM spans s
FROM (
SELECT
*,
row_number() OVER (PARTITION BY id ORDER BY last_updated_at DESC) AS latest
FROM spans
WHERE id IN (SELECT id FROM span_ids)
) AS s
LEFT JOIN comments_final AS c ON s.id = c.entity_id
WHERE s.id IN (SELECT id FROM span_ids)
WHERE s.latest = 1
GROUP BY
s.*,
duration_millis
Expand Down
6 changes: 3 additions & 3 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,14 +435,14 @@ AND id in (
if(end_time IS NOT NULL AND start_time IS NOT NULL
AND notEquals(start_time, toDateTime64('1970-01-01 00:00:00.000', 9)),
(dateDiff('microsecond', start_time, end_time) / 1000.0),
NULL) AS duration_millis
NULL) AS duration_millis,
row_number() OVER (PARTITION BY id ORDER BY last_updated_at DESC) AS latest
FROM traces
WHERE id IN (SELECT id FROM traces_ids)
ORDER BY id DESC, last_updated_at DESC
LIMIT 1 BY id
) AS t
LEFT JOIN span_usage AS s ON t.id = s.trace_id
LEFT JOIN comments_final AS c ON t.id = c.entity_id
WHERE t.latest = 1
thiagohora marked this conversation as resolved.
Show resolved Hide resolved
GROUP BY
t.*,
t.duration_millis
Expand Down
Loading