-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[OPIK-2946][BE][FE] add full span details when adding to dataset item #3964
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.
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-2946][BE][FE] add full span details when adding to dataset item #3964
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends the functionality for adding enriched items to datasets from spans, complementing the previously implemented feature for traces (PR #3910). It introduces a new backend endpoint and frontend UI for adding spans to datasets with configurable metadata enrichment options.
Key changes:
- Added backend endpoint to create dataset items from spans with enrichment options
- Implemented frontend UI for span-specific metadata configuration
- Created span enrichment service mirroring trace enrichment functionality
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AddToDatasetDialog.tsx |
Refactored to support both trace and span enrichment, extracted reusable components for metadata configuration |
useAddSpansToDatasetMutation.ts |
New API hook for adding spans to datasets with enrichment options |
SpanEnrichmentServiceTest.java |
Comprehensive unit tests for span enrichment service covering various option combinations |
DatasetsResourceCreateFromSpansTest.java |
Integration tests for the new spans-to-dataset endpoint |
DatasetResourceClient.java |
Added client methods for calling the new spans endpoint |
EnrichmentUtils.java |
Extracted common enrichment utilities for building feedback scores and comments JSON nodes |
TraceEnrichmentService.java |
Refactored to use shared enrichment utilities |
SpanService.java |
Added getByIds method for batch span retrieval |
SpanEnrichmentService.java |
New service implementing span enrichment logic parallel to trace enrichment |
SpanDAO.java |
Updated to support batch retrieval of spans by IDs |
DatasetItemService.java |
Added createFromSpans method for creating dataset items from spans |
DatasetsResource.java |
New REST endpoint for creating dataset items from spans |
CreateDatasetItemsFromSpansRequest.java |
Request object for the new spans endpoint |
apps/opik-frontend/src/components/pages-shared/traces/AddToDatasetDialog/AddToDatasetDialog.tsx
Outdated
Show resolved
Hide resolved
SDK E2E Tests Results0 tests 0 ✅ 0s ⏱️ Results for commit 52270ad. |
|
✅ Test environment is now available! Access Information
The deployment has completed successfully and the version has been verified. |
Backend Tests Results5 433 tests 5 426 ✅ 52m 4s ⏱️ Results for commit 742c941. ♻️ This comment has been updated with latest results. |
andriidudar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Ido, thanks for adding this functionality for Spans as well!
The code looks good overall — I’ve just left a few comments that I believe should be addressed to help improve performance and reduce some unnecessary load on the backend.
apps/opik-frontend/src/components/pages-shared/traces/AddToDatasetDialog/AddToDatasetDialog.tsx
Outdated
Show resolved
Hide resolved
apps/opik-frontend/src/components/pages-shared/traces/AddToDatasetDialog/AddToDatasetDialog.tsx
Outdated
Show resolved
Hide resolved
apps/opik-frontend/src/components/pages-shared/traces/AddToDatasetDialog/AddToDatasetDialog.tsx
Outdated
Show resolved
Hide resolved
andriidudar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @idoberko2 , Thanks for handling my comments.
Approved
Details
This completes the work done in #3910. In the previous PR, this was implemented for traces. This PR extends this functionality to spans.
add_full_span_to_dataset_items.mov
Change checklist
Issues
Testing
Documentation
Not changing as the relevant changes were made in #3910.